From bc55574a1628a73f5378670248e995b8cd97f029 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Thu, 7 Dec 2023 16:36:09 +0000 Subject: [PATCH 01/21] use StateAtBlock and reference states when recreating --- arbitrum/apibackend.go | 52 ++++++++++++++++-------- arbitrum/recreatestate.go | 21 ---------- eth/api_backend.go | 27 ++++++------ eth/catalyst/api_test.go | 3 +- eth/state_accessor.go | 14 +++++-- graphql/graphql.go | 18 ++++---- internal/ethapi/api.go | 30 +++++++++----- internal/ethapi/api_test.go | 6 +-- internal/ethapi/backend.go | 8 +++- internal/ethapi/transaction_args_test.go | 8 ++-- les/api_backend.go | 19 +++++---- 11 files changed, 116 insertions(+), 90 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 4419efce60..baa7238fee 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -229,10 +229,11 @@ func (a *APIBackend) FeeHistory( // use the most recent average compute rate for all blocks // note: while we could query this value for each block, it'd be prohibitively expensive - state, _, err := a.StateAndHeaderByNumber(ctx, newestBlock) + state, _, release, err := a.StateAndHeaderByNumber(ctx, newestBlock) if err != nil { return common.Big0, nil, nil, nil, err } + defer release() speedLimit, err := core.GetArbOSSpeedLimitPerSecond(state) if err != nil { return common.Big0, nil, nil, nil, err @@ -433,40 +434,59 @@ func (a *APIBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc. return nil, errors.New("invalid arguments; neither block nor hash specified") } -func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types.Header, err error) (*state.StateDB, *types.Header, error) { +func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types.Header, err error) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { if err != nil { - return nil, header, err + return nil, header, nil, err } if header == nil { - return nil, nil, errors.New("header not found") + return nil, nil, nil, errors.New("header not found") } if !a.BlockChain().Config().IsArbitrumNitro(header.Number) { - return nil, header, types.ErrUseFallback + return nil, header, nil, types.ErrUseFallback } bc := a.BlockChain() stateFor := func(header *types.Header) (*state.StateDB, error) { - return bc.StateAt(header.Root) + if header.Root != (common.Hash{}) { + // Try referencing the root, if it isn't in dirties cache then Reference will have no effect + bc.StateCache().TrieDB().Reference(header.Root, common.Hash{}) + } + state, err := bc.StateAt(header.Root) + return state, err } - state, lastHeader, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) + lastState, lastHeader, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) if err != nil { - return nil, nil, err + return nil, nil, nil, err + } + release := func() { + if lastHeader.Root != (common.Hash{}) { + bc.StateCache().TrieDB().Dereference(lastHeader.Root) + } } if lastHeader == header { - return state, header, nil + return lastState, header, release, nil } - state, err = AdvanceStateUpToBlock(ctx, bc, state, header, lastHeader, nil) - if err != nil { - return nil, nil, err + defer release() + targetBlock := bc.GetBlockByNumber(header.Number.Uint64()) + if targetBlock == nil { + return nil, nil, nil, errors.New("target block not found") + } + lastBlock := bc.GetBlockByNumber(lastHeader.Number.Uint64()) + if lastBlock == nil { + return nil, nil, nil, errors.New("last block not found") } - return state, header, err + reexec := uint64(0) + checkLive := false + preferDisk := true + state, release, err := eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, targetBlock, reexec, lastState, lastBlock, checkLive, preferDisk) + return state, header, release, err } -func (a *APIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { +func (a *APIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { header, err := a.HeaderByNumber(ctx, number) return a.stateAndHeaderFromHeader(ctx, header, err) } -func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { +func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { header, err := a.HeaderByNumberOrHash(ctx, blockNrOrHash) return a.stateAndHeaderFromHeader(ctx, header, err) } @@ -476,7 +496,7 @@ func (a *APIBackend) StateAtBlock(ctx context.Context, block *types.Block, reexe return nil, nil, types.ErrUseFallback } // DEV: This assumes that `StateAtBlock` only accesses the blockchain and chainDb fields - return eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, block, reexec, base, checkLive, preferDisk) + return eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, block, reexec, base, nil, checkLive, preferDisk) } func (a *APIBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*core.Message, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) { diff --git a/arbitrum/recreatestate.go b/arbitrum/recreatestate.go index 1b2dbb1a18..e44262cdca 100644 --- a/arbitrum/recreatestate.go +++ b/arbitrum/recreatestate.go @@ -80,24 +80,3 @@ func AdvanceStateByBlock(ctx context.Context, bc *core.BlockChain, state *state. } return state, block, nil } - -func AdvanceStateUpToBlock(ctx context.Context, bc *core.BlockChain, state *state.StateDB, targetHeader *types.Header, lastAvailableHeader *types.Header, logFunc StateBuildingLogFunction) (*state.StateDB, error) { - returnedBlockNumber := targetHeader.Number.Uint64() - blockToRecreate := lastAvailableHeader.Number.Uint64() + 1 - prevHash := lastAvailableHeader.Hash() - for ctx.Err() == nil { - state, block, err := AdvanceStateByBlock(ctx, bc, state, targetHeader, blockToRecreate, prevHash, logFunc) - if err != nil { - return nil, err - } - prevHash = block.Hash() - if blockToRecreate >= returnedBlockNumber { - if block.Hash() != targetHeader.Hash() { - return nil, fmt.Errorf("blockHash doesn't match when recreating number: %d expected: %v got: %v", blockToRecreate, targetHeader.Hash(), block.Hash()) - } - return state, nil - } - blockToRecreate++ - } - return nil, ctx.Err() -} diff --git a/eth/api_backend.go b/eth/api_backend.go index 77be44defc..558ded2e98 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -37,6 +37,7 @@ import ( "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" + "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/miner" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -198,46 +199,46 @@ func (b *EthAPIBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) return b.eth.miner.PendingBlockAndReceipts() } -func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { +func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { // Pending state is only known by the miner if number == rpc.PendingBlockNumber { block, state := b.eth.miner.Pending() if block == nil || state == nil { - return nil, nil, errors.New("pending state is not available") + return nil, nil, nil, errors.New("pending state is not available") } - return state, block.Header(), nil + return state, block.Header(), ethapi.NoOpStateRelease, nil } // Otherwise resolve the block number and return its state header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if header == nil { - return nil, nil, errors.New("header not found") + return nil, nil, nil, errors.New("header not found") } stateDb, err := b.eth.BlockChain().StateAt(header.Root) - return stateDb, header, err + return stateDb, header, ethapi.NoOpStateRelease, err } -func (b *EthAPIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { +func (b *EthAPIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } if hash, ok := blockNrOrHash.Hash(); ok { header, err := b.HeaderByHash(ctx, hash) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if header == nil { - return nil, nil, errors.New("header for hash not found") + return nil, nil, nil, errors.New("header for hash not found") } if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash { - return nil, nil, errors.New("hash is not currently canonical") + return nil, nil, nil, errors.New("hash is not currently canonical") } stateDb, err := b.eth.BlockChain().StateAt(header.Root) - return stateDb, header, err + return stateDb, header, ethapi.NoOpStateRelease, err } - return nil, nil, errors.New("invalid arguments; neither block nor hash specified") + return nil, nil, nil, errors.New("invalid arguments; neither block nor hash specified") } func (b *EthAPIBackend) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { @@ -423,7 +424,7 @@ func (b *EthAPIBackend) StartMining() error { } func (b *EthAPIBackend) StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, tracers.StateReleaseFunc, error) { - return b.eth.StateAtBlock(ctx, block, reexec, base, readOnly, preferDisk) + return b.eth.StateAtBlock(ctx, block, reexec, base, nil, readOnly, preferDisk) } func (b *EthAPIBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*core.Message, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) { diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 05ad3def48..cd846a499e 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -1143,10 +1143,11 @@ func TestWithdrawals(t *testing.T) { } // 11: verify withdrawals were processed. - db, _, err := ethservice.APIBackend.StateAndHeaderByNumber(context.Background(), rpc.BlockNumber(execData.ExecutionPayload.Number)) + db, _, release, err := ethservice.APIBackend.StateAndHeaderByNumber(context.Background(), rpc.BlockNumber(execData.ExecutionPayload.Number)) if err != nil { t.Fatalf("unable to load db: %v", err) } + defer release() for i, w := range blockParams.Withdrawals { // w.Amount is in gwei, balance in wei if db.GetBalance(w.Address).Uint64() != w.Amount*params.GWei { diff --git a/eth/state_accessor.go b/eth/state_accessor.go index ff18d27054..cf780de7f1 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -53,7 +53,7 @@ import ( // - preferDisk: this arg can be used by the caller to signal that even though the 'base' is // provided, it would be preferable to start from a fresh state, if we have it // on disk. -func (eth *Ethereum) StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) { +func (eth *Ethereum) StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, baseBlock *types.Block, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) { var ( current *types.Block database state.Database @@ -66,8 +66,10 @@ func (eth *Ethereum) StateAtBlock(ctx context.Context, block *types.Block, reexe // The state is available in live database, create a reference // on top to prevent garbage collection and return a release // function to deref it. + + // Try referencing the root, if it isn't in dirties cache then Reference will have no effect + statedb.Database().TrieDB().Reference(block.Root(), common.Hash{}) if statedb, err = eth.blockchain.StateAt(block.Root()); err == nil { - statedb.Database().TrieDB().Reference(block.Root(), common.Hash{}) return statedb, func() { statedb.Database().TrieDB().Dereference(block.Root()) }, nil @@ -95,7 +97,11 @@ func (eth *Ethereum) StateAtBlock(ctx context.Context, block *types.Block, reexe } // The optional base statedb is given, mark the start point as parent block statedb, database, report = base, base.Database(), false - current = eth.blockchain.GetBlock(block.ParentHash(), block.NumberU64()-1) + if baseBlock == nil { + current = eth.blockchain.GetBlock(block.ParentHash(), block.NumberU64()-1) + } else { + current = baseBlock + } } else { // Otherwise, try to reexec blocks until we find a state or reach our limit current = block @@ -214,7 +220,7 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, } // Lookup the statedb of parent block from the live database, // otherwise regenerate it on the flight. - statedb, release, err := eth.StateAtBlock(ctx, parent, reexec, nil, true, false) + statedb, release, err := eth.StateAtBlock(ctx, parent, reexec, nil, nil, true, false) if err != nil { return nil, vm.BlockContext{}, nil, nil, err } diff --git a/graphql/graphql.go b/graphql/graphql.go index 5eea340f66..ac8b79ea5a 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -86,9 +86,9 @@ type Account struct { } // getState fetches the StateDB object for an account. -func (a *Account) getState(ctx context.Context) (*state.StateDB, error) { - state, _, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) - return state, err +func (a *Account) getState(ctx context.Context) (*state.StateDB, ethapi.StateReleaseFunc, error) { + state, _, release, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) + return state, release, err } func (a *Account) Address(ctx context.Context) (common.Address, error) { @@ -96,10 +96,11 @@ func (a *Account) Address(ctx context.Context) (common.Address, error) { } func (a *Account) Balance(ctx context.Context) (hexutil.Big, error) { - state, err := a.getState(ctx) + state, release, err := a.getState(ctx) if err != nil { return hexutil.Big{}, err } + defer release() balance := state.GetBalance(a.address) if balance == nil { return hexutil.Big{}, fmt.Errorf("failed to load balance %x", a.address) @@ -116,26 +117,29 @@ func (a *Account) TransactionCount(ctx context.Context) (hexutil.Uint64, error) } return hexutil.Uint64(nonce), nil } - state, err := a.getState(ctx) + state, release, err := a.getState(ctx) if err != nil { return 0, err } + defer release() return hexutil.Uint64(state.GetNonce(a.address)), nil } func (a *Account) Code(ctx context.Context) (hexutil.Bytes, error) { - state, err := a.getState(ctx) + state, release, err := a.getState(ctx) if err != nil { return hexutil.Bytes{}, err } + defer release() return state.GetCode(a.address), nil } func (a *Account) Storage(ctx context.Context, args struct{ Slot common.Hash }) (common.Hash, error) { - state, err := a.getState(ctx) + state, release, err := a.getState(ctx) if err != nil { return common.Hash{}, err } + defer release() return state.GetState(a.address, args.Slot), nil } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 1fd916d903..c268e08586 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -620,7 +620,7 @@ func (s *BlockChainAPI) BlockNumber() hexutil.Uint64 { // given block number. The rpc.LatestBlockNumber and rpc.PendingBlockNumber meta // block numbers are also allowed. func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, blockNrOrHash rpc.BlockNumberOrHash) (*hexutil.Big, error) { - state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Big @@ -629,6 +629,7 @@ func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, } return nil, err } + defer release() return (*hexutil.Big)(state.GetBalance(address)), state.Error() } @@ -681,10 +682,11 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st } } - state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err } + defer release() if storageTrie, err = state.StorageTrie(address); err != nil { return nil, err } @@ -867,7 +869,7 @@ func (s *BlockChainAPI) GetUncleCountByBlockHash(ctx context.Context, blockHash // GetCode returns the code stored at the given address in the state for the given block number. func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) { - state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Bytes @@ -876,6 +878,7 @@ func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blo } return nil, err } + defer release() code := state.GetCode(address) return code, state.Error() } @@ -888,7 +891,7 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address if err != nil { return nil, fmt.Errorf("unable to decode storage key: %s", err) } - state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Bytes @@ -897,6 +900,7 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address } return nil, err } + defer release() res := state.GetState(address, key) return res[:], state.Error() } @@ -1125,10 +1129,11 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64, runMode core.MessageRunMode) (*core.ExecutionResult, error) { defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now()) - state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err } + defer release() return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, runMode) } @@ -1232,10 +1237,11 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr } // Recap the highest gas limit with account's available balance. if feeCap.BitLen() != 0 { - state, _, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if err != nil { return 0, err } + defer release() err = overrides.Apply(state) if err != nil { return 0, err @@ -1265,10 +1271,11 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr // Arbitrum: raise the gas cap to ignore L1 costs so that it's compute-only vanillaGasCap := gasCap { - state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return 0, err } + defer release() gasCap, err = args.L2OnlyGasCap(gasCap, header, state, core.MessageGasEstimationMode) if err != nil { return 0, err @@ -1295,10 +1302,11 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr } return result.Failed(), result, nil } - state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return 0, err } + defer release() err = overrides.Apply(state) if err != nil { return 0, err @@ -1695,10 +1703,11 @@ func (s *BlockChainAPI) CreateAccessList(ctx context.Context, args TransactionAr // If the transaction itself fails, an vmErr is returned. func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrHash, args TransactionArgs) (acl types.AccessList, gasUsed uint64, vmErr error, err error) { // Retrieve the execution context - db, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + db, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if db == nil || err != nil { return nil, 0, nil, err } + defer release() // If the gas amount is not set, default to RPC gas cap. if args.Gas == nil { tmp := hexutil.Uint64(b.RPCGasCap()) @@ -1829,7 +1838,7 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common return (*hexutil.Uint64)(&nonce), nil } // Resolve block number and use its state to ask for the nonce - state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Uint64 @@ -1838,6 +1847,7 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common } return nil, err } + defer release() nonce := state.GetNonce(address) return (*hexutil.Uint64)(&nonce), state.Error() } diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index dd810a9a96..cf87c553d0 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -437,7 +437,7 @@ func (b testBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc. func (b testBackend) GetBody(ctx context.Context, hash common.Hash, number rpc.BlockNumber) (*types.Body, error) { return b.chain.GetBlock(hash, uint64(number.Int64())).Body(), nil } -func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { +func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) { if number == rpc.PendingBlockNumber { panic("pending state not implemented") } @@ -449,9 +449,9 @@ func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.Bloc return nil, nil, errors.New("header not found") } stateDb, err := b.chain.StateAt(header.Root) - return stateDb, header, err + return stateDb, header, NoOpStateRelease, err } -func (b testBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { +func (b testBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index 6ebd22ddaf..7017b8c010 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -37,6 +37,10 @@ import ( "github.com/ethereum/go-ethereum/rpc" ) +type StateReleaseFunc func() + +var NoOpStateRelease StateReleaseFunc = func() {} + // Backend interface provides the common API services (that are provided by // both full and light clients) with access to necessary functions. type Backend interface { @@ -66,8 +70,8 @@ type Backend interface { BlockByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Block, error) BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Block, error) - StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) - StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) + StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) + StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) PendingBlockAndReceipts() (*types.Block, types.Receipts) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) GetTd(ctx context.Context, hash common.Hash) *big.Int diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index ba5dd3977f..bc75c3dc12 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -291,11 +291,11 @@ func (b *backendMock) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc func (b *backendMock) GetBody(ctx context.Context, hash common.Hash, number rpc.BlockNumber) (*types.Body, error) { return nil, nil } -func (b *backendMock) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { - return nil, nil, nil +func (b *backendMock) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) { + return nil, nil, NoOpStateRelease, nil } -func (b *backendMock) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { - return nil, nil, nil +func (b *backendMock) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) { + return nil, nil, NoOpStateRelease, nil } func (b *backendMock) PendingBlockAndReceipts() (*types.Block, types.Receipts) { return nil, nil } func (b *backendMock) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { diff --git a/les/api_backend.go b/les/api_backend.go index 311db0b828..323c97a9c3 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -36,6 +36,7 @@ import ( "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" + "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/light" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -137,32 +138,32 @@ func (b *LesApiBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) return nil, nil } -func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { +func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if header == nil { - return nil, nil, errors.New("header not found") + return nil, nil, nil, errors.New("header not found") } - return light.NewState(ctx, header, b.eth.odr), header, nil + return light.NewState(ctx, header, b.eth.odr), header, ethapi.NoOpStateRelease, nil } -func (b *LesApiBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { +func (b *LesApiBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } if hash, ok := blockNrOrHash.Hash(); ok { header := b.eth.blockchain.GetHeaderByHash(hash) if header == nil { - return nil, nil, errors.New("header for hash not found") + return nil, nil, nil, errors.New("header for hash not found") } if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash { - return nil, nil, errors.New("hash is not currently canonical") + return nil, nil, nil, errors.New("hash is not currently canonical") } - return light.NewState(ctx, header, b.eth.odr), header, nil + return light.NewState(ctx, header, b.eth.odr), header, ethapi.NoOpStateRelease, nil } - return nil, nil, errors.New("invalid arguments; neither block nor hash specified") + return nil, nil, nil, errors.New("invalid arguments; neither block nor hash specified") } func (b *LesApiBackend) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { From 0227c54150444a4fe111d9fcab08f696ff98b816 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Thu, 7 Dec 2023 16:46:58 +0000 Subject: [PATCH 02/21] fix ethapi test --- internal/ethapi/api_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index cf87c553d0..dad135b79e 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -443,10 +443,10 @@ func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.Bloc } header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if header == nil { - return nil, nil, errors.New("header not found") + return nil, nil, nil, errors.New("header not found") } stateDb, err := b.chain.StateAt(header.Root) return stateDb, header, NoOpStateRelease, err From 8d5951aa69681b7a35c7a1c5ce1f1d6f74ff60d1 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Mon, 11 Dec 2023 18:46:54 +0000 Subject: [PATCH 03/21] add baseBlock comment, fix referencing befor StateAt --- eth/state_accessor.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eth/state_accessor.go b/eth/state_accessor.go index cf780de7f1..a440a8a8df 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -47,6 +47,7 @@ import ( // - reexec: The maximum number of blocks to reprocess trying to obtain the desired state // - base: If the caller is tracing multiple blocks, the caller can provide the parent // state continuously from the callsite. +// - baseBlock: Arbitrum specific: caller can provide the block from which reprocessing should start. Previous argument (base) is assumed to be the state at the block. If base is not provided, baseBlock is ignored. // - readOnly: If true, then the live 'blockchain' state database is used. No mutation should // be made from caller, e.g. perform Commit or other 'save-to-disk' changes. // Otherwise, the trash generated by caller may be persisted permanently. @@ -68,7 +69,7 @@ func (eth *Ethereum) StateAtBlock(ctx context.Context, block *types.Block, reexe // function to deref it. // Try referencing the root, if it isn't in dirties cache then Reference will have no effect - statedb.Database().TrieDB().Reference(block.Root(), common.Hash{}) + eth.blockchain.StateCache().TrieDB().Reference(block.Root(), common.Hash{}) if statedb, err = eth.blockchain.StateAt(block.Root()); err == nil { return statedb, func() { statedb.Database().TrieDB().Dereference(block.Root()) From b158011a68df2450cbb1e9aa617abbe95ca0c879 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Mon, 18 Dec 2023 16:01:37 +0000 Subject: [PATCH 04/21] use finalizer instead of returning state release function --- arbitrum/apibackend.go | 62 +++++++++++++++--------- eth/api_backend.go | 25 +++++----- eth/catalyst/api_test.go | 3 +- graphql/graphql.go | 18 +++---- internal/ethapi/api.go | 30 ++++-------- internal/ethapi/api_test.go | 6 +-- internal/ethapi/backend.go | 8 +-- internal/ethapi/transaction_args_test.go | 8 +-- les/api_backend.go | 15 +++--- 9 files changed, 86 insertions(+), 89 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index baa7238fee..b7b9f5731b 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/big" + "runtime" "strconv" "strings" "time" @@ -229,11 +230,10 @@ func (a *APIBackend) FeeHistory( // use the most recent average compute rate for all blocks // note: while we could query this value for each block, it'd be prohibitively expensive - state, _, release, err := a.StateAndHeaderByNumber(ctx, newestBlock) + state, _, err := a.StateAndHeaderByNumber(ctx, newestBlock) if err != nil { return common.Big0, nil, nil, nil, err } - defer release() speedLimit, err := core.GetArbOSSpeedLimitPerSecond(state) if err != nil { return common.Big0, nil, nil, nil, err @@ -434,15 +434,15 @@ func (a *APIBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc. return nil, errors.New("invalid arguments; neither block nor hash specified") } -func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types.Header, err error) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types.Header, err error) (*state.StateDB, *types.Header, error) { if err != nil { - return nil, header, nil, err + return nil, header, err } if header == nil { - return nil, nil, nil, errors.New("header not found") + return nil, nil, errors.New("header not found") } if !a.BlockChain().Config().IsArbitrumNitro(header.Number) { - return nil, header, nil, types.ErrUseFallback + return nil, header, types.ErrUseFallback } bc := a.BlockChain() stateFor := func(header *types.Header) (*state.StateDB, error) { @@ -450,43 +450,61 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types // Try referencing the root, if it isn't in dirties cache then Reference will have no effect bc.StateCache().TrieDB().Reference(header.Root, common.Hash{}) } - state, err := bc.StateAt(header.Root) - return state, err + statedb, err := state.New(header.Root, bc.StateCache(), bc.Snapshots()) + if err != nil { + return nil, err + } + if header.Root != (common.Hash{}) { + // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream + headerRoot := header.Root + runtime.SetFinalizer(statedb, func(_ *state.StateDB) { + bc.StateCache().TrieDB().Dereference(headerRoot) + }) + } + return statedb, err } lastState, lastHeader, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) if err != nil { - return nil, nil, nil, err - } - release := func() { - if lastHeader.Root != (common.Hash{}) { - bc.StateCache().TrieDB().Dereference(lastHeader.Root) - } + return nil, nil, err } if lastHeader == header { - return lastState, header, release, nil + return lastState, header, nil + } + if lastHeader.Root != (common.Hash{}) { + defer bc.StateCache().TrieDB().Dereference(lastHeader.Root) } - defer release() targetBlock := bc.GetBlockByNumber(header.Number.Uint64()) if targetBlock == nil { - return nil, nil, nil, errors.New("target block not found") + return nil, nil, errors.New("target block not found") } lastBlock := bc.GetBlockByNumber(lastHeader.Number.Uint64()) if lastBlock == nil { - return nil, nil, nil, errors.New("last block not found") + return nil, nil, errors.New("last block not found") } reexec := uint64(0) checkLive := false preferDisk := true - state, release, err := eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, targetBlock, reexec, lastState, lastBlock, checkLive, preferDisk) - return state, header, release, err + statedb, release, err := eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, targetBlock, reexec, lastState, lastBlock, checkLive, preferDisk) + if err != nil { + return nil, nil, err + } + // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream + // to set a finalizer we need to allocated the obj in current block + statedb, err = state.New(header.Root, statedb.Database(), nil) + if header.Root != (common.Hash{}) { + runtime.SetFinalizer(statedb, func(_ *state.StateDB) { + release() + }) + } + return statedb, header, err } -func (a *APIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (a *APIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { header, err := a.HeaderByNumber(ctx, number) return a.stateAndHeaderFromHeader(ctx, header, err) } -func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { header, err := a.HeaderByNumberOrHash(ctx, blockNrOrHash) return a.stateAndHeaderFromHeader(ctx, header, err) } diff --git a/eth/api_backend.go b/eth/api_backend.go index 558ded2e98..7656d63c58 100644 --- a/eth/api_backend.go +++ b/eth/api_backend.go @@ -37,7 +37,6 @@ import ( "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" - "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/miner" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -199,46 +198,46 @@ func (b *EthAPIBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) return b.eth.miner.PendingBlockAndReceipts() } -func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (b *EthAPIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { // Pending state is only known by the miner if number == rpc.PendingBlockNumber { block, state := b.eth.miner.Pending() if block == nil || state == nil { - return nil, nil, nil, errors.New("pending state is not available") + return nil, nil, errors.New("pending state is not available") } - return state, block.Header(), ethapi.NoOpStateRelease, nil + return state, block.Header(), nil } // Otherwise resolve the block number and return its state header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if header == nil { - return nil, nil, nil, errors.New("header not found") + return nil, nil, errors.New("header not found") } stateDb, err := b.eth.BlockChain().StateAt(header.Root) - return stateDb, header, ethapi.NoOpStateRelease, err + return stateDb, header, err } -func (b *EthAPIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (b *EthAPIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } if hash, ok := blockNrOrHash.Hash(); ok { header, err := b.HeaderByHash(ctx, hash) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if header == nil { - return nil, nil, nil, errors.New("header for hash not found") + return nil, nil, errors.New("header for hash not found") } if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash { - return nil, nil, nil, errors.New("hash is not currently canonical") + return nil, nil, errors.New("hash is not currently canonical") } stateDb, err := b.eth.BlockChain().StateAt(header.Root) - return stateDb, header, ethapi.NoOpStateRelease, err + return stateDb, header, err } - return nil, nil, nil, errors.New("invalid arguments; neither block nor hash specified") + return nil, nil, errors.New("invalid arguments; neither block nor hash specified") } func (b *EthAPIBackend) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index cd846a499e..05ad3def48 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -1143,11 +1143,10 @@ func TestWithdrawals(t *testing.T) { } // 11: verify withdrawals were processed. - db, _, release, err := ethservice.APIBackend.StateAndHeaderByNumber(context.Background(), rpc.BlockNumber(execData.ExecutionPayload.Number)) + db, _, err := ethservice.APIBackend.StateAndHeaderByNumber(context.Background(), rpc.BlockNumber(execData.ExecutionPayload.Number)) if err != nil { t.Fatalf("unable to load db: %v", err) } - defer release() for i, w := range blockParams.Withdrawals { // w.Amount is in gwei, balance in wei if db.GetBalance(w.Address).Uint64() != w.Amount*params.GWei { diff --git a/graphql/graphql.go b/graphql/graphql.go index ac8b79ea5a..5eea340f66 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -86,9 +86,9 @@ type Account struct { } // getState fetches the StateDB object for an account. -func (a *Account) getState(ctx context.Context) (*state.StateDB, ethapi.StateReleaseFunc, error) { - state, _, release, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) - return state, release, err +func (a *Account) getState(ctx context.Context) (*state.StateDB, error) { + state, _, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) + return state, err } func (a *Account) Address(ctx context.Context) (common.Address, error) { @@ -96,11 +96,10 @@ func (a *Account) Address(ctx context.Context) (common.Address, error) { } func (a *Account) Balance(ctx context.Context) (hexutil.Big, error) { - state, release, err := a.getState(ctx) + state, err := a.getState(ctx) if err != nil { return hexutil.Big{}, err } - defer release() balance := state.GetBalance(a.address) if balance == nil { return hexutil.Big{}, fmt.Errorf("failed to load balance %x", a.address) @@ -117,29 +116,26 @@ func (a *Account) TransactionCount(ctx context.Context) (hexutil.Uint64, error) } return hexutil.Uint64(nonce), nil } - state, release, err := a.getState(ctx) + state, err := a.getState(ctx) if err != nil { return 0, err } - defer release() return hexutil.Uint64(state.GetNonce(a.address)), nil } func (a *Account) Code(ctx context.Context) (hexutil.Bytes, error) { - state, release, err := a.getState(ctx) + state, err := a.getState(ctx) if err != nil { return hexutil.Bytes{}, err } - defer release() return state.GetCode(a.address), nil } func (a *Account) Storage(ctx context.Context, args struct{ Slot common.Hash }) (common.Hash, error) { - state, release, err := a.getState(ctx) + state, err := a.getState(ctx) if err != nil { return common.Hash{}, err } - defer release() return state.GetState(a.address, args.Slot), nil } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index c268e08586..1fd916d903 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -620,7 +620,7 @@ func (s *BlockChainAPI) BlockNumber() hexutil.Uint64 { // given block number. The rpc.LatestBlockNumber and rpc.PendingBlockNumber meta // block numbers are also allowed. func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, blockNrOrHash rpc.BlockNumberOrHash) (*hexutil.Big, error) { - state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Big @@ -629,7 +629,6 @@ func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, } return nil, err } - defer release() return (*hexutil.Big)(state.GetBalance(address)), state.Error() } @@ -682,11 +681,10 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st } } - state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err } - defer release() if storageTrie, err = state.StorageTrie(address); err != nil { return nil, err } @@ -869,7 +867,7 @@ func (s *BlockChainAPI) GetUncleCountByBlockHash(ctx context.Context, blockHash // GetCode returns the code stored at the given address in the state for the given block number. func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blockNrOrHash rpc.BlockNumberOrHash) (hexutil.Bytes, error) { - state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Bytes @@ -878,7 +876,6 @@ func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blo } return nil, err } - defer release() code := state.GetCode(address) return code, state.Error() } @@ -891,7 +888,7 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address if err != nil { return nil, fmt.Errorf("unable to decode storage key: %s", err) } - state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Bytes @@ -900,7 +897,6 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address } return nil, err } - defer release() res := state.GetState(address, key) return res[:], state.Error() } @@ -1129,11 +1125,10 @@ func doCall(ctx context.Context, b Backend, args TransactionArgs, state *state.S func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides, timeout time.Duration, globalGasCap uint64, runMode core.MessageRunMode) (*core.ExecutionResult, error) { defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now()) - state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return nil, err } - defer release() return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, runMode) } @@ -1237,11 +1232,10 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr } // Recap the highest gas limit with account's available balance. if feeCap.BitLen() != 0 { - state, _, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if err != nil { return 0, err } - defer release() err = overrides.Apply(state) if err != nil { return 0, err @@ -1271,11 +1265,10 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr // Arbitrum: raise the gas cap to ignore L1 costs so that it's compute-only vanillaGasCap := gasCap { - state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return 0, err } - defer release() gasCap, err = args.L2OnlyGasCap(gasCap, header, state, core.MessageGasEstimationMode) if err != nil { return 0, err @@ -1302,11 +1295,10 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr } return result.Failed(), result, nil } - state, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { return 0, err } - defer release() err = overrides.Apply(state) if err != nil { return 0, err @@ -1703,11 +1695,10 @@ func (s *BlockChainAPI) CreateAccessList(ctx context.Context, args TransactionAr // If the transaction itself fails, an vmErr is returned. func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrHash, args TransactionArgs) (acl types.AccessList, gasUsed uint64, vmErr error, err error) { // Retrieve the execution context - db, header, release, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + db, header, err := b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if db == nil || err != nil { return nil, 0, nil, err } - defer release() // If the gas amount is not set, default to RPC gas cap. if args.Gas == nil { tmp := hexutil.Uint64(b.RPCGasCap()) @@ -1838,7 +1829,7 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common return (*hexutil.Uint64)(&nonce), nil } // Resolve block number and use its state to ask for the nonce - state, _, release, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) + state, _, err := s.b.StateAndHeaderByNumberOrHash(ctx, blockNrOrHash) if state == nil || err != nil { if client := fallbackClientFor(s.b, err); client != nil { var res hexutil.Uint64 @@ -1847,7 +1838,6 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common } return nil, err } - defer release() nonce := state.GetNonce(address) return (*hexutil.Uint64)(&nonce), state.Error() } diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index dad135b79e..c92b438b03 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -437,7 +437,7 @@ func (b testBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc. func (b testBackend) GetBody(ctx context.Context, hash common.Hash, number rpc.BlockNumber) (*types.Body, error) { return b.chain.GetBlock(hash, uint64(number.Int64())).Body(), nil } -func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) { +func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { if number == rpc.PendingBlockNumber { panic("pending state not implemented") } @@ -449,9 +449,9 @@ func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.Bloc return nil, nil, nil, errors.New("header not found") } stateDb, err := b.chain.StateAt(header.Root) - return stateDb, header, NoOpStateRelease, err + return stateDb, header, err } -func (b testBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) { +func (b testBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } diff --git a/internal/ethapi/backend.go b/internal/ethapi/backend.go index 7017b8c010..6ebd22ddaf 100644 --- a/internal/ethapi/backend.go +++ b/internal/ethapi/backend.go @@ -37,10 +37,6 @@ import ( "github.com/ethereum/go-ethereum/rpc" ) -type StateReleaseFunc func() - -var NoOpStateRelease StateReleaseFunc = func() {} - // Backend interface provides the common API services (that are provided by // both full and light clients) with access to necessary functions. type Backend interface { @@ -70,8 +66,8 @@ type Backend interface { BlockByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Block, error) BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Block, error) - StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) - StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) + StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) + StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) PendingBlockAndReceipts() (*types.Block, types.Receipts) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) GetTd(ctx context.Context, hash common.Hash) *big.Int diff --git a/internal/ethapi/transaction_args_test.go b/internal/ethapi/transaction_args_test.go index bc75c3dc12..ba5dd3977f 100644 --- a/internal/ethapi/transaction_args_test.go +++ b/internal/ethapi/transaction_args_test.go @@ -291,11 +291,11 @@ func (b *backendMock) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc func (b *backendMock) GetBody(ctx context.Context, hash common.Hash, number rpc.BlockNumber) (*types.Body, error) { return nil, nil } -func (b *backendMock) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, StateReleaseFunc, error) { - return nil, nil, NoOpStateRelease, nil +func (b *backendMock) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { + return nil, nil, nil } -func (b *backendMock) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, StateReleaseFunc, error) { - return nil, nil, NoOpStateRelease, nil +func (b *backendMock) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { + return nil, nil, nil } func (b *backendMock) PendingBlockAndReceipts() (*types.Block, types.Receipts) { return nil, nil } func (b *backendMock) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { diff --git a/les/api_backend.go b/les/api_backend.go index 323c97a9c3..1ac58f9380 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -36,7 +36,6 @@ import ( "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/ethdb" "github.com/ethereum/go-ethereum/event" - "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/light" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" @@ -138,7 +137,7 @@ func (b *LesApiBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) return nil, nil } -func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { header, err := b.HeaderByNumber(ctx, number) if err != nil { return nil, nil, nil, err @@ -146,24 +145,24 @@ func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.B if header == nil { return nil, nil, nil, errors.New("header not found") } - return light.NewState(ctx, header, b.eth.odr), header, ethapi.NoOpStateRelease, nil + return light.NewState(ctx, header, b.eth.odr), header, nil } -func (b *LesApiBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, ethapi.StateReleaseFunc, error) { +func (b *LesApiBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { if blockNr, ok := blockNrOrHash.Number(); ok { return b.StateAndHeaderByNumber(ctx, blockNr) } if hash, ok := blockNrOrHash.Hash(); ok { header := b.eth.blockchain.GetHeaderByHash(hash) if header == nil { - return nil, nil, nil, errors.New("header for hash not found") + return nil, nil, errors.New("header for hash not found") } if blockNrOrHash.RequireCanonical && b.eth.blockchain.GetCanonicalHash(header.Number.Uint64()) != hash { - return nil, nil, nil, errors.New("hash is not currently canonical") + return nil, nil, errors.New("hash is not currently canonical") } - return light.NewState(ctx, header, b.eth.odr), header, ethapi.NoOpStateRelease, nil + return light.NewState(ctx, header, b.eth.odr), header, nil } - return nil, nil, nil, errors.New("invalid arguments; neither block nor hash specified") + return nil, nil, errors.New("invalid arguments; neither block nor hash specified") } func (b *LesApiBackend) GetReceipts(ctx context.Context, hash common.Hash) (types.Receipts, error) { From 8186f88540091efe44b7afcdbad0f35a2ef3c1a0 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Tue, 19 Dec 2023 13:47:31 +0000 Subject: [PATCH 05/21] clean up extra return values --- internal/ethapi/api_test.go | 4 ++-- les/api_backend.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index c92b438b03..dd810a9a96 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -443,10 +443,10 @@ func (b testBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.Bloc } header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if header == nil { - return nil, nil, nil, errors.New("header not found") + return nil, nil, errors.New("header not found") } stateDb, err := b.chain.StateAt(header.Root) return stateDb, header, err diff --git a/les/api_backend.go b/les/api_backend.go index 1ac58f9380..311db0b828 100644 --- a/les/api_backend.go +++ b/les/api_backend.go @@ -140,10 +140,10 @@ func (b *LesApiBackend) PendingBlockAndReceipts() (*types.Block, types.Receipts) func (b *LesApiBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*state.StateDB, *types.Header, error) { header, err := b.HeaderByNumber(ctx, number) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if header == nil { - return nil, nil, nil, errors.New("header not found") + return nil, nil, errors.New("header not found") } return light.NewState(ctx, header, b.eth.odr), header, nil } From 5645cf052e8ea8c3491c3a20fcbf57282495ac46 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 12 Jan 2024 20:53:34 +0000 Subject: [PATCH 06/21] set statedb finalizer only in stateAndHeaderFromHeader --- arbitrum/apibackend.go | 23 ++++++++++------------- arbitrum/recordingdb.go | 7 ++++++- arbitrum/recreatestate.go | 29 ++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index b7b9f5731b..b7bbcab1bd 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -445,34 +445,33 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types return nil, header, types.ErrUseFallback } bc := a.BlockChain() - stateFor := func(header *types.Header) (*state.StateDB, error) { + stateFor := func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { if header.Root != (common.Hash{}) { // Try referencing the root, if it isn't in dirties cache then Reference will have no effect bc.StateCache().TrieDB().Reference(header.Root, common.Hash{}) } statedb, err := state.New(header.Root, bc.StateCache(), bc.Snapshots()) if err != nil { - return nil, err + return nil, noopStateRelease, err } if header.Root != (common.Hash{}) { - // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream headerRoot := header.Root - runtime.SetFinalizer(statedb, func(_ *state.StateDB) { - bc.StateCache().TrieDB().Dereference(headerRoot) - }) + return statedb, func() { bc.StateCache().TrieDB().Dereference(headerRoot) }, nil } - return statedb, err + return statedb, noopStateRelease, nil } - lastState, lastHeader, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) + lastState, lastHeader, lastStateRelease, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) if err != nil { return nil, nil, err } if lastHeader == header { + // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream + runtime.SetFinalizer(lastState, func(_ *state.StateDB) { + lastStateRelease() + }) return lastState, header, nil } - if lastHeader.Root != (common.Hash{}) { - defer bc.StateCache().TrieDB().Dereference(lastHeader.Root) - } + defer lastStateRelease() targetBlock := bc.GetBlockByNumber(header.Number.Uint64()) if targetBlock == nil { return nil, nil, errors.New("target block not found") @@ -489,8 +488,6 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types return nil, nil, err } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream - // to set a finalizer we need to allocated the obj in current block - statedb, err = state.New(header.Root, statedb.Database(), nil) if header.Root != (common.Hash{}) { runtime.SetFinalizer(statedb, func(_ *state.StateDB) { release() diff --git a/arbitrum/recordingdb.go b/arbitrum/recordingdb.go index a5c08b99a8..5534ee10e1 100644 --- a/arbitrum/recordingdb.go +++ b/arbitrum/recordingdb.go @@ -302,7 +302,12 @@ func (r *RecordingDatabase) PreimagesFromRecording(chainContextIf core.ChainCont } func (r *RecordingDatabase) GetOrRecreateState(ctx context.Context, header *types.Header, logFunc StateBuildingLogFunction) (*state.StateDB, error) { - state, currentHeader, err := FindLastAvailableState(ctx, r.bc, r.StateFor, header, logFunc, -1) + stateFor := func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { + state, err := r.StateFor(header) + // we don't use the release functor pattern here yet + return state, noopStateRelease, err + } + state, currentHeader, _, err := FindLastAvailableState(ctx, r.bc, stateFor, header, logFunc, -1) if err != nil { return nil, err } diff --git a/arbitrum/recreatestate.go b/arbitrum/recreatestate.go index e44262cdca..9966074211 100644 --- a/arbitrum/recreatestate.go +++ b/arbitrum/recreatestate.go @@ -9,6 +9,7 @@ import ( "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/eth/tracers" "github.com/pkg/errors" ) @@ -16,51 +17,61 @@ var ( ErrDepthLimitExceeded = errors.New("state recreation l2 gas depth limit exceeded") ) +type StateReleaseFunc tracers.StateReleaseFunc + +var noopStateRelease StateReleaseFunc = func() {} + type StateBuildingLogFunction func(targetHeader, header *types.Header, hasState bool) -type StateForHeaderFunction func(header *types.Header) (*state.StateDB, error) +type StateForHeaderFunction func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) // finds last available state and header checking it first for targetHeader then looking backwards // if maxDepthInL2Gas is positive, it constitutes a limit for cumulative l2 gas used of the traversed blocks // else if maxDepthInL2Gas is -1, the traversal depth is not limited // otherwise only targetHeader state is checked and no search is performed -func FindLastAvailableState(ctx context.Context, bc *core.BlockChain, stateFor StateForHeaderFunction, targetHeader *types.Header, logFunc StateBuildingLogFunction, maxDepthInL2Gas int64) (*state.StateDB, *types.Header, error) { +func FindLastAvailableState(ctx context.Context, bc *core.BlockChain, stateFor StateForHeaderFunction, targetHeader *types.Header, logFunc StateBuildingLogFunction, maxDepthInL2Gas int64) (*state.StateDB, *types.Header, StateReleaseFunc, error) { genesis := bc.Config().ArbitrumChainParams.GenesisBlockNum currentHeader := targetHeader var state *state.StateDB var err error var l2GasUsed uint64 + release := noopStateRelease for ctx.Err() == nil { lastHeader := currentHeader - state, err = stateFor(currentHeader) + state, release, err = stateFor(currentHeader) if err == nil { break } if maxDepthInL2Gas > 0 { receipts := bc.GetReceiptsByHash(currentHeader.Hash()) if receipts == nil { - return nil, lastHeader, fmt.Errorf("failed to get receipts for hash %v", currentHeader.Hash()) + release() + return nil, lastHeader, nil, fmt.Errorf("failed to get receipts for hash %v", currentHeader.Hash()) } for _, receipt := range receipts { l2GasUsed += receipt.GasUsed - receipt.GasUsedForL1 } if l2GasUsed > uint64(maxDepthInL2Gas) { - return nil, lastHeader, ErrDepthLimitExceeded + release() + return nil, lastHeader, nil, ErrDepthLimitExceeded } } else if maxDepthInL2Gas != InfiniteMaxRecreateStateDepth { - return nil, lastHeader, err + release() + return nil, lastHeader, nil, err } if logFunc != nil { logFunc(targetHeader, currentHeader, false) } if currentHeader.Number.Uint64() <= genesis { - return nil, lastHeader, errors.Wrap(err, fmt.Sprintf("moved beyond genesis looking for state %d, genesis %d", targetHeader.Number.Uint64(), genesis)) + release() + return nil, lastHeader, nil, errors.Wrap(err, fmt.Sprintf("moved beyond genesis looking for state %d, genesis %d", targetHeader.Number.Uint64(), genesis)) } currentHeader = bc.GetHeader(currentHeader.ParentHash, currentHeader.Number.Uint64()-1) if currentHeader == nil { - return nil, lastHeader, fmt.Errorf("chain doesn't contain parent of block %d hash %v", lastHeader.Number, lastHeader.Hash()) + release() + return nil, lastHeader, nil, fmt.Errorf("chain doesn't contain parent of block %d hash %v", lastHeader.Number, lastHeader.Hash()) } } - return state, currentHeader, ctx.Err() + return state, currentHeader, release, ctx.Err() } func AdvanceStateByBlock(ctx context.Context, bc *core.BlockChain, state *state.StateDB, targetHeader *types.Header, blockToRecreate uint64, prevBlockHash common.Hash, logFunc StateBuildingLogFunction) (*state.StateDB, *types.Block, error) { From ce1438bda1ab97ed3c228ac275fccd47cf5eb2f9 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Mon, 5 Feb 2024 14:41:02 +0000 Subject: [PATCH 07/21] save some states from triegc when stoping sparse archive node (same as for full node) --- core/blockchain.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 9a09b42c1d..3ec18b57ef 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1074,7 +1074,8 @@ func (bc *BlockChain) Stop() { // - HEAD: So we don't need to reprocess any blocks in the general case // - HEAD-1: So we don't do large reorgs if our HEAD becomes an uncle // - HEAD-127: So we have a hard limit on the number of blocks reexecuted - if !bc.cacheConfig.TrieDirtyDisabled { + // It applies for both full node and sparse archive node + if !bc.cacheConfig.TrieDirtyDisabled || bc.cacheConfig.MaxNumberOfBlocksToSkipStateSaving > 0 || bc.cacheConfig.MaxAmountOfGasToSkipStateSaving > 0 { triedb := bc.triedb for _, offset := range []uint64{0, 1, bc.cacheConfig.TriesInMemory - 1, math.MaxUint64} { @@ -1499,7 +1500,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. return nil } // If we're running an archive node, flush - // If MaxNumberOfBlocksToSkipStateSaving or MaxAmountOfGasToSkipStateSaving is not zero, then flushing of some blocks will be skipped: + // Sparse archive: if MaxNumberOfBlocksToSkipStateSaving or MaxAmountOfGasToSkipStateSaving is not zero, then flushing of some blocks will be skipped: // * at most MaxNumberOfBlocksToSkipStateSaving block state commits will be skipped // * sum of gas used in skipped blocks will be at most MaxAmountOfGasToSkipStateSaving archiveNode := bc.cacheConfig.TrieDirtyDisabled @@ -1529,7 +1530,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. // we are skipping saving the trie to diskdb, so we need to keep the trie in memory and garbage collect it later } - // Full node or archive node that's not keeping all states, do proper garbage collection + // Full node or sparse archive node that's not keeping all states, do proper garbage collection bc.triedb.Reference(root, common.Hash{}) // metadata reference to keep trie alive bc.triegc.Push(trieGcEntry{root, block.Header().Time}, -int64(block.NumberU64())) From 5177f70f4b6fa4b63bb446d239f91827a0fde231 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Tue, 6 Feb 2024 15:20:03 +0000 Subject: [PATCH 08/21] add debug logs with state finalizers counters --- arbitrum/apibackend.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 5b9ec8becd..2801b4402c 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -8,6 +8,7 @@ import ( "runtime" "strconv" "strings" + "sync/atomic" "time" "github.com/ethereum/go-ethereum" @@ -38,6 +39,10 @@ type APIBackend struct { fallbackClient types.FallbackClient sync SyncProgressBackend + + // TODO remove + liveStateFinalizers atomic.Int64 + recreatedStateFinalizers atomic.Int64 } type timeoutFallbackClient struct { @@ -466,7 +471,11 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } if lastHeader == header { // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream + a.liveStateFinalizers.Add(1) + log.Debug("Live state finalizer set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) runtime.SetFinalizer(lastState, func(_ *state.StateDB) { + a.liveStateFinalizers.Add(-1) + log.Debug("Live state finalizer called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) lastStateRelease() }) return lastState, header, nil @@ -489,7 +498,12 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream if header.Root != (common.Hash{}) { + a.recreatedStateFinalizers.Add(1) + log.Debug("Recreated state finalizer set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) runtime.SetFinalizer(statedb, func(_ *state.StateDB) { + // TODO remove + a.recreatedStateFinalizers.Add(-1) + log.Debug("Recreated state finalizer called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) release() }) } From 97a51030390fd8443557d4faae4e74032aaea33b Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Thu, 8 Feb 2024 20:08:01 +0000 Subject: [PATCH 09/21] fix baseBlock usage in StateAtBlock --- eth/state_accessor.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/eth/state_accessor.go b/eth/state_accessor.go index 940dd6c103..812cd8b367 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -63,7 +63,18 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u // The state is both for reading and writing, or it's unavailable in disk, // try to construct/recover the state over an ephemeral trie.Database for // isolating the live one. - if base != nil { + if baseBlock != nil { + // Create an ephemeral trie.Database for isolating the live one + triedb = trie.NewDatabase(eth.chainDb, trie.HashDefaults) + database = state.NewDatabaseWithNodeDB(eth.chainDb, triedb) + // there is no need of referencing baseBlock state as it's read from disk + statedb, err = state.New(baseBlock.Root(), database, nil) + if err == nil { + return statedb, noopReleaser, nil + } + report = false + current = baseBlock + } else if base != nil { if preferDisk { // Create an ephemeral trie.Database for isolating the live one. Otherwise // the internal junks created by tracing will be persisted into the disk. @@ -77,11 +88,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u } // The optional base statedb is given, mark the start point as parent block statedb, database, triedb, report = base, base.Database(), base.Database().TrieDB(), false - if baseBlock == nil { - current = eth.blockchain.GetBlock(block.ParentHash(), block.NumberU64()-1) - } else { - current = baseBlock - } + current = eth.blockchain.GetBlock(block.ParentHash(), block.NumberU64()-1) } else { // Otherwise, try to reexec blocks until we find a state or reach our limit current = block @@ -209,8 +216,7 @@ func (eth *Ethereum) pathState(block *types.Block) (*state.StateDB, func(), erro // - base: If the caller is tracing multiple blocks, the caller can provide the parent // state continuously from the callsite. // - baseBlock: Arbitrum specific: caller can provide the block from which reprocessing should -// start. Previous argument (base) is assumed to be the state at the block. If base is not -// provided, baseBlock is ignored. +// start, if baseBlock is provided then base parameter is ignored // - readOnly: If true, then the live 'blockchain' state database is used. No mutation should // be made from caller, e.g. perform Commit or other 'save-to-disk' changes. // Otherwise, the trash generated by caller may be persisted permanently. From 65e4c8a20256e01e9cd6c4dc347540db18c215c3 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 9 Feb 2024 16:43:36 +0000 Subject: [PATCH 10/21] add state release method to StateDB --- arbitrum/apibackend.go | 17 ++++++++------- arbitrum/recreatestate.go | 5 ----- core/state/statedb.go | 46 +++++++++++++++++++++++++++++++++++++++ eth/state_accessor.go | 4 ++-- graphql/graphql.go | 3 +++ internal/ethapi/api.go | 9 ++++++++ 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 2801b4402c..6f3ec55a78 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "math/big" - "runtime" "strconv" "strings" "sync/atomic" @@ -240,6 +239,7 @@ func (a *APIBackend) FeeHistory( return common.Big0, nil, nil, nil, err } speedLimit, err := core.GetArbOSSpeedLimitPerSecond(state) + state.Release() if err != nil { return common.Big0, nil, nil, nil, err } @@ -472,12 +472,13 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types if lastHeader == header { // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream a.liveStateFinalizers.Add(1) - log.Debug("Live state finalizer set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) - runtime.SetFinalizer(lastState, func(_ *state.StateDB) { + lastState.SetRelease(func() { a.liveStateFinalizers.Add(-1) - log.Debug("Live state finalizer called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + // TODO remove logs and counters + log.Debug("Live state relase called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) lastStateRelease() }) + log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return lastState, header, nil } defer lastStateRelease() @@ -499,13 +500,13 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream if header.Root != (common.Hash{}) { a.recreatedStateFinalizers.Add(1) - log.Debug("Recreated state finalizer set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) - runtime.SetFinalizer(statedb, func(_ *state.StateDB) { - // TODO remove + statedb.SetRelease(func() { + // TODO remove logs and counters a.recreatedStateFinalizers.Add(-1) - log.Debug("Recreated state finalizer called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + log.Debug("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) release() }) + log.Debug("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) } return statedb, header, err } diff --git a/arbitrum/recreatestate.go b/arbitrum/recreatestate.go index 9966074211..7d10c9c9e8 100644 --- a/arbitrum/recreatestate.go +++ b/arbitrum/recreatestate.go @@ -44,30 +44,25 @@ func FindLastAvailableState(ctx context.Context, bc *core.BlockChain, stateFor S if maxDepthInL2Gas > 0 { receipts := bc.GetReceiptsByHash(currentHeader.Hash()) if receipts == nil { - release() return nil, lastHeader, nil, fmt.Errorf("failed to get receipts for hash %v", currentHeader.Hash()) } for _, receipt := range receipts { l2GasUsed += receipt.GasUsed - receipt.GasUsedForL1 } if l2GasUsed > uint64(maxDepthInL2Gas) { - release() return nil, lastHeader, nil, ErrDepthLimitExceeded } } else if maxDepthInL2Gas != InfiniteMaxRecreateStateDepth { - release() return nil, lastHeader, nil, err } if logFunc != nil { logFunc(targetHeader, currentHeader, false) } if currentHeader.Number.Uint64() <= genesis { - release() return nil, lastHeader, nil, errors.Wrap(err, fmt.Sprintf("moved beyond genesis looking for state %d, genesis %d", targetHeader.Number.Uint64(), genesis)) } currentHeader = bc.GetHeader(currentHeader.ParentHash, currentHeader.Number.Uint64()-1) if currentHeader == nil { - release() return nil, lastHeader, nil, fmt.Errorf("chain doesn't contain parent of block %d hash %v", lastHeader.Number, lastHeader.Hash()) } } diff --git a/core/state/statedb.go b/core/state/statedb.go index d2e1227a69..8cc0ae5685 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -21,7 +21,9 @@ import ( "bytes" "fmt" "math/big" + "runtime" "sort" + "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -145,9 +147,36 @@ type StateDB struct { // Testing hooks onCommit func(states *triestate.Set) // Hook invoked when commit is performed + // arbitrum: reference counted cleanup hook + release func() + releaseRefCount *atomic.Int64 + released bool + deterministic bool } +func (s *StateDB) SetRelease(release func()) { + if s.release == nil { + s.release = release + s.releaseRefCount = new(atomic.Int64) + s.releaseRefCount.Add(1) + } else { + // TODO + panic("StateDB.SetRelease called more then once") + } +} + +func (s *StateDB) Release() { + if s.release != nil && !s.released { + if ref := s.releaseRefCount.Add(-1); ref == 0 { + s.release() + } else { + log.Warn("statedb not release, refCount != 0", ref) + } + s.released = true + } +} + // New creates a new state from a given trie. func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) { tr, err := db.OpenTrie(root) @@ -175,7 +204,14 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) accessList: newAccessList(), transientStorage: newTransientStorage(), hasher: crypto.NewKeccakState(), + + release: nil, + releaseRefCount: nil, + released: false, } + runtime.SetFinalizer(sdb, func(s *StateDB) { + s.Release() + }) if sdb.snaps != nil { sdb.snap = sdb.snaps.Snapshot(root) } @@ -752,7 +788,17 @@ func (s *StateDB) Copy() *StateDB { // miner to operate trie-backed only. snaps: s.snaps, snap: s.snap, + + release: s.release, + releaseRefCount: s.releaseRefCount, + released: false, + } + if s.releaseRefCount != nil { + s.releaseRefCount.Add(1) } + runtime.SetFinalizer(state, func(s *StateDB) { + s.Release() + }) // Copy the dirty states, logs, and preimages for addr := range s.journal.dirties { // As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527), diff --git a/eth/state_accessor.go b/eth/state_accessor.go index 812cd8b367..9a0a49f411 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -69,8 +69,8 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u database = state.NewDatabaseWithNodeDB(eth.chainDb, triedb) // there is no need of referencing baseBlock state as it's read from disk statedb, err = state.New(baseBlock.Root(), database, nil) - if err == nil { - return statedb, noopReleaser, nil + if err != nil { + return nil, nil, fmt.Errorf("state for base block missing: %w", err) } report = false current = baseBlock diff --git a/graphql/graphql.go b/graphql/graphql.go index ae4e5314d4..86ef409110 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -88,6 +88,9 @@ type Account struct { // getState fetches the StateDB object for an account. func (a *Account) getState(ctx context.Context) (*state.StateDB, error) { state, _, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) + if state != nil && err == nil { + defer state.Release() + } return state, err } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 87a18f131b..2eb718f2e4 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -630,6 +630,7 @@ func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, } return nil, err } + defer state.Release() return (*hexutil.Big)(state.GetBalance(address)), state.Error() } @@ -686,6 +687,7 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st if state == nil || err != nil { return nil, err } + defer state.Release() if storageRoot := state.GetStorageRoot(address); storageRoot != types.EmptyRootHash && storageRoot != (common.Hash{}) { id := trie.StorageTrieID(header.Root, crypto.Keccak256Hash(address.Bytes()), storageRoot) tr, err := trie.NewStateTrie(id, state.Database().TrieDB()) @@ -884,6 +886,7 @@ func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blo } return nil, err } + defer state.Release() code := state.GetCode(address) return code, state.Error() } @@ -905,6 +908,7 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address } return nil, err } + defer state.Release() res := state.GetState(address, key) return res[:], state.Error() } @@ -1186,6 +1190,7 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash if state == nil || err != nil { return nil, err } + defer state.Release() header = updateHeaderForPendingBlocks(blockNrOrHash, header) return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, runMode) @@ -1315,6 +1320,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr if state == nil || err != nil { return 0, err } + defer state.Release() if err := overrides.Apply(state); err != nil { return 0, err } @@ -1351,6 +1357,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr if state == nil || err != nil { return 0, err } + defer state.Release() gasCap, err = args.L2OnlyGasCap(gasCap, header, state, core.MessageGasEstimationMode) if err != nil { return 0, err @@ -1806,6 +1813,7 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH if db == nil || err != nil { return nil, 0, nil, err } + defer db.Release() // If the gas amount is not set, default to RPC gas cap. if args.Gas == nil { tmp := hexutil.Uint64(b.RPCGasCap()) @@ -1945,6 +1953,7 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common } return nil, err } + defer state.Release() nonce := state.GetNonce(address) return (*hexutil.Uint64)(&nonce), state.Error() } From d1db8eb8f16ccc13eb56add844ed0404b6292e6a Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 9 Feb 2024 16:45:39 +0000 Subject: [PATCH 11/21] add todo comment --- core/state/statedb.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 8cc0ae5685..361ce0707e 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -171,7 +171,8 @@ func (s *StateDB) Release() { if ref := s.releaseRefCount.Add(-1); ref == 0 { s.release() } else { - log.Warn("statedb not release, refCount != 0", ref) + //TODO remove + log.Warn("statedb not released, refCount != 0", ref) } s.released = true } From f1fff92aa346aa8458b2de4d555fa1fbb5e1e877 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 16 Feb 2024 14:47:15 +0000 Subject: [PATCH 12/21] fix isolation of live state database --- arbitrum/apibackend.go | 57 +++++++++++++++++++++++++++++------------- core/state/statedb.go | 2 +- eth/state_accessor.go | 11 +------- 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 6f3ec55a78..46cabb6bf4 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -15,6 +15,7 @@ import ( "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" @@ -23,6 +24,7 @@ import ( "github.com/ethereum/go-ethereum/core/bloombits" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/state" + "github.com/ethereum/go-ethereum/core/state/snapshot" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/eth/filters" @@ -450,32 +452,53 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types return nil, header, types.ErrUseFallback } bc := a.BlockChain() - stateFor := func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { - if header.Root != (common.Hash{}) { - // Try referencing the root, if it isn't in dirties cache then Reference will have no effect - bc.StateCache().TrieDB().Reference(header.Root, common.Hash{}) - } - statedb, err := state.New(header.Root, bc.StateCache(), bc.Snapshots()) - if err != nil { - return nil, noopStateRelease, err - } - if header.Root != (common.Hash{}) { - headerRoot := header.Root - return statedb, func() { bc.StateCache().TrieDB().Dereference(headerRoot) }, nil + stateFor := func(db state.Database, snapshots *snapshot.Tree) func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { + return func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { + if header.Root != (common.Hash{}) { + // Try referencing the root, if it isn't in dirties cache then Reference will have no effect + db.TrieDB().Reference(header.Root, common.Hash{}) + } + statedb, err := state.New(header.Root, db, snapshots) + if err != nil { + return nil, nil, err + } + if header.Root != (common.Hash{}) { + headerRoot := header.Root + return statedb, func() { db.TrieDB().Dereference(headerRoot) }, nil + } + return statedb, noopStateRelease, nil } - return statedb, noopStateRelease, nil } - lastState, lastHeader, lastStateRelease, err := FindLastAvailableState(ctx, bc, stateFor, header, nil, a.b.config.MaxRecreateStateDepth) + liveState, liveStateRelease, err := stateFor(bc.StateCache(), bc.Snapshots())(header) + if err == nil { + a.liveStateFinalizers.Add(1) + liveState.SetRelease(func() { + a.liveStateFinalizers.Add(-1) + // TODO remove logs and counters + log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + liveStateRelease() + }) + log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + return liveState, header, nil + } + // else err != nil => we don't need to call liveStateRelease + + // Create an ephemeral trie.Database for isolating the live one + // note: triedb cleans cache is disabled in trie.HashDefaults + // note: only states committed to diskdb can be found as we're creating new triedb + // note: snapshots are not used here + ephemeral := state.NewDatabaseWithConfig(a.ChainDb(), trie.HashDefaults) + lastState, lastHeader, lastStateRelease, err := FindLastAvailableState(ctx, bc, stateFor(ephemeral, nil), header, nil, a.b.config.MaxRecreateStateDepth) if err != nil { return nil, nil, err } + // make sure that we haven't found the state in diskdb if lastHeader == header { - // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream a.liveStateFinalizers.Add(1) lastState.SetRelease(func() { a.liveStateFinalizers.Add(-1) // TODO remove logs and counters - log.Debug("Live state relase called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) lastStateRelease() }) log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) @@ -492,7 +515,7 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } reexec := uint64(0) checkLive := false - preferDisk := true + preferDisk := false // preferDisk is ignored in this case statedb, release, err := eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, targetBlock, reexec, lastState, lastBlock, checkLive, preferDisk) if err != nil { return nil, nil, err diff --git a/core/state/statedb.go b/core/state/statedb.go index 361ce0707e..da65123e6d 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -172,7 +172,7 @@ func (s *StateDB) Release() { s.release() } else { //TODO remove - log.Warn("statedb not released, refCount != 0", ref) + log.Warn("statedb not released, refCount != 0", "refcnt", ref) } s.released = true } diff --git a/eth/state_accessor.go b/eth/state_accessor.go index 9a0a49f411..b96a585ffd 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -64,16 +64,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u // try to construct/recover the state over an ephemeral trie.Database for // isolating the live one. if baseBlock != nil { - // Create an ephemeral trie.Database for isolating the live one - triedb = trie.NewDatabase(eth.chainDb, trie.HashDefaults) - database = state.NewDatabaseWithNodeDB(eth.chainDb, triedb) - // there is no need of referencing baseBlock state as it's read from disk - statedb, err = state.New(baseBlock.Root(), database, nil) - if err != nil { - return nil, nil, fmt.Errorf("state for base block missing: %w", err) - } - report = false - current = baseBlock + current, statedb, database, triedb, report = baseBlock, base, base.Database(), base.Database().TrieDB(), false } else if base != nil { if preferDisk { // Create an ephemeral trie.Database for isolating the live one. Otherwise From e10de759ea54fb8e759d004cde5d21b037af32cb Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 16 Feb 2024 15:34:52 +0000 Subject: [PATCH 13/21] arbitrum/apibackend: add live and ephemeral states metrics --- arbitrum/apibackend.go | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 46cabb6bf4..0a329da2cf 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -15,6 +15,7 @@ import ( "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/trie" "github.com/ethereum/go-ethereum/accounts" @@ -35,6 +36,13 @@ import ( "github.com/ethereum/go-ethereum/rpc" ) +var ( + referencedLiveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/referenced", nil) + releasedLiveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/released", nil) + referencedEphemermalStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/ephemeral/referenced", nil) + releasedEphemermalStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/ephemeral/released", nil) +) + type APIBackend struct { b *Backend @@ -471,12 +479,12 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } liveState, liveStateRelease, err := stateFor(bc.StateCache(), bc.Snapshots())(header) if err == nil { - a.liveStateFinalizers.Add(1) + referencedLiveStatesCounter.Inc(1) liveState.SetRelease(func() { - a.liveStateFinalizers.Add(-1) // TODO remove logs and counters log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) liveStateRelease() + releasedLiveStatesCounter.Inc(1) }) log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return liveState, header, nil @@ -494,12 +502,12 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } // make sure that we haven't found the state in diskdb if lastHeader == header { - a.liveStateFinalizers.Add(1) + referencedEphemermalStatesCounter.Inc(1) lastState.SetRelease(func() { - a.liveStateFinalizers.Add(-1) // TODO remove logs and counters log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) lastStateRelease() + releasedEphemermalStatesCounter.Inc(1) }) log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return lastState, header, nil @@ -518,19 +526,18 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types preferDisk := false // preferDisk is ignored in this case statedb, release, err := eth.NewArbEthereum(a.b.arb.BlockChain(), a.ChainDb()).StateAtBlock(ctx, targetBlock, reexec, lastState, lastBlock, checkLive, preferDisk) if err != nil { - return nil, nil, err + return nil, nil, fmt.Errorf("failed to recreate state: %w", err) } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream - if header.Root != (common.Hash{}) { - a.recreatedStateFinalizers.Add(1) - statedb.SetRelease(func() { - // TODO remove logs and counters - a.recreatedStateFinalizers.Add(-1) - log.Debug("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) - release() - }) - log.Debug("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) - } + referencedEphemermalStatesCounter.Inc(1) + statedb.SetRelease(func() { + // TODO remove logs and counters + a.recreatedStateFinalizers.Add(-1) + log.Debug("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + release() + releasedEphemermalStatesCounter.Inc(1) + }) + log.Debug("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return statedb, header, err } From cf61e26fb7c6b81eb8a1782543ecec489b498faa Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 21 Feb 2024 12:48:18 +0000 Subject: [PATCH 14/21] update state recreation metrics --- arbitrum/apibackend.go | 28 ++++++++-------------------- eth/state_accessor.go | 11 ++++++++++- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 0a329da2cf..8d74000e4f 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -37,10 +37,8 @@ import ( ) var ( - referencedLiveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/referenced", nil) - releasedLiveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/released", nil) - referencedEphemermalStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/ephemeral/referenced", nil) - releasedEphemermalStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/ephemeral/released", nil) + liveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live", nil) + recreatedStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/recreated", nil) ) type APIBackend struct { @@ -49,8 +47,6 @@ type APIBackend struct { fallbackClient types.FallbackClient sync SyncProgressBackend - // TODO remove - liveStateFinalizers atomic.Int64 recreatedStateFinalizers atomic.Int64 } @@ -479,14 +475,10 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } liveState, liveStateRelease, err := stateFor(bc.StateCache(), bc.Snapshots())(header) if err == nil { - referencedLiveStatesCounter.Inc(1) + liveStatesCounter.Inc(1) liveState.SetRelease(func() { - // TODO remove logs and counters - log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) liveStateRelease() - releasedLiveStatesCounter.Inc(1) }) - log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return liveState, header, nil } // else err != nil => we don't need to call liveStateRelease @@ -502,14 +494,10 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } // make sure that we haven't found the state in diskdb if lastHeader == header { - referencedEphemermalStatesCounter.Inc(1) + liveStatesCounter.Inc(1) lastState.SetRelease(func() { - // TODO remove logs and counters - log.Debug("Live state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) lastStateRelease() - releasedEphemermalStatesCounter.Inc(1) }) - log.Debug("Live state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) return lastState, header, nil } defer lastStateRelease() @@ -529,15 +517,15 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types return nil, nil, fmt.Errorf("failed to recreate state: %w", err) } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream - referencedEphemermalStatesCounter.Inc(1) + recreatedStatesCounter.Inc(1) + a.recreatedStateFinalizers.Add(1) statedb.SetRelease(func() { // TODO remove logs and counters a.recreatedStateFinalizers.Add(-1) - log.Debug("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + log.Warn("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load()) release() - releasedEphemermalStatesCounter.Inc(1) }) - log.Debug("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load(), "liveStateFinalizers", a.liveStateFinalizers.Load()) + log.Warn("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load()) return statedb, header, err } diff --git a/eth/state_accessor.go b/eth/state_accessor.go index b96a585ffd..439d88c2d8 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -30,9 +30,15 @@ import ( "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/metrics" "github.com/ethereum/go-ethereum/trie" ) +var ( + recreatedStatesCounter = metrics.NewRegisteredCounter("eth/stateaccessor/recreated/states", nil) + recreatedBytesMeter = metrics.NewRegisteredMeter("eth/stateaccessor/recreated/bytes", nil) +) + // noopReleaser is returned in case there is no operation expected // for releasing state. var noopReleaser = tracers.StateReleaseFunc(func() {}) @@ -171,10 +177,13 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u } parent = root } + _, nodes, imgs := triedb.Size() // all memory is contained within the nodes return in hashdb if report { - _, nodes, imgs := triedb.Size() // all memory is contained within the nodes return in hashdb log.Info("Historical state regenerated", "block", current.NumberU64(), "elapsed", time.Since(start), "nodes", nodes, "preimages", imgs) } + recreatedStatesCounter.Inc(1) + recreatedBytesMeter.Mark(int64(nodes)) + return statedb, func() { triedb.Dereference(block.Root()) }, nil } From 27e28ede0056e5cb13b6fd97313781e8ef1b853f Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Thu, 22 Feb 2024 00:31:16 +0000 Subject: [PATCH 15/21] simplify StateDB.Release, don't set finalizer as it keeps StateDB live too long --- core/state/statedb.go | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index da65123e6d..d771609ba3 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -21,9 +21,7 @@ import ( "bytes" "fmt" "math/big" - "runtime" "sort" - "sync/atomic" "time" "github.com/ethereum/go-ethereum/common" @@ -147,10 +145,9 @@ type StateDB struct { // Testing hooks onCommit func(states *triestate.Set) // Hook invoked when commit is performed - // arbitrum: reference counted cleanup hook - release func() - releaseRefCount *atomic.Int64 - released bool + // arbitrum: cleanup hook + release func() + released bool deterministic bool } @@ -158,8 +155,6 @@ type StateDB struct { func (s *StateDB) SetRelease(release func()) { if s.release == nil { s.release = release - s.releaseRefCount = new(atomic.Int64) - s.releaseRefCount.Add(1) } else { // TODO panic("StateDB.SetRelease called more then once") @@ -168,12 +163,7 @@ func (s *StateDB) SetRelease(release func()) { func (s *StateDB) Release() { if s.release != nil && !s.released { - if ref := s.releaseRefCount.Add(-1); ref == 0 { - s.release() - } else { - //TODO remove - log.Warn("statedb not released, refCount != 0", "refcnt", ref) - } + s.release() s.released = true } } @@ -206,13 +196,9 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) transientStorage: newTransientStorage(), hasher: crypto.NewKeccakState(), - release: nil, - releaseRefCount: nil, - released: false, + release: nil, + released: false, } - runtime.SetFinalizer(sdb, func(s *StateDB) { - s.Release() - }) if sdb.snaps != nil { sdb.snap = sdb.snaps.Snapshot(root) } @@ -789,17 +775,7 @@ func (s *StateDB) Copy() *StateDB { // miner to operate trie-backed only. snaps: s.snaps, snap: s.snap, - - release: s.release, - releaseRefCount: s.releaseRefCount, - released: false, - } - if s.releaseRefCount != nil { - s.releaseRefCount.Add(1) } - runtime.SetFinalizer(state, func(s *StateDB) { - s.Release() - }) // Copy the dirty states, logs, and preimages for addr := range s.journal.dirties { // As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527), From 3e1f80f784e2f4e2109b80a4e214cb029086dc26 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 23 Feb 2024 10:50:13 +0000 Subject: [PATCH 16/21] bring back AdvanceStateUpToBlock --- arbitrum/apibackend.go | 2 +- arbitrum/recordingdb.go | 2 +- arbitrum/recreatestate.go | 25 +++++++++++++++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 8d74000e4f..4dd7bbec6c 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -470,7 +470,7 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types headerRoot := header.Root return statedb, func() { db.TrieDB().Dereference(headerRoot) }, nil } - return statedb, noopStateRelease, nil + return statedb, NoopStateRelease, nil } } liveState, liveStateRelease, err := stateFor(bc.StateCache(), bc.Snapshots())(header) diff --git a/arbitrum/recordingdb.go b/arbitrum/recordingdb.go index 7d89b14c6d..30cafe5a77 100644 --- a/arbitrum/recordingdb.go +++ b/arbitrum/recordingdb.go @@ -312,7 +312,7 @@ func (r *RecordingDatabase) GetOrRecreateState(ctx context.Context, header *type stateFor := func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) { state, err := r.StateFor(header) // we don't use the release functor pattern here yet - return state, noopStateRelease, err + return state, NoopStateRelease, err } state, currentHeader, _, err := FindLastAvailableState(ctx, r.bc, stateFor, header, logFunc, -1) if err != nil { diff --git a/arbitrum/recreatestate.go b/arbitrum/recreatestate.go index 7d10c9c9e8..bd16c9d3a9 100644 --- a/arbitrum/recreatestate.go +++ b/arbitrum/recreatestate.go @@ -19,7 +19,7 @@ var ( type StateReleaseFunc tracers.StateReleaseFunc -var noopStateRelease StateReleaseFunc = func() {} +var NoopStateRelease StateReleaseFunc = func() {} type StateBuildingLogFunction func(targetHeader, header *types.Header, hasState bool) type StateForHeaderFunction func(header *types.Header) (*state.StateDB, StateReleaseFunc, error) @@ -34,7 +34,7 @@ func FindLastAvailableState(ctx context.Context, bc *core.BlockChain, stateFor S var state *state.StateDB var err error var l2GasUsed uint64 - release := noopStateRelease + release := NoopStateRelease for ctx.Err() == nil { lastHeader := currentHeader state, release, err = stateFor(currentHeader) @@ -86,3 +86,24 @@ func AdvanceStateByBlock(ctx context.Context, bc *core.BlockChain, state *state. } return state, block, nil } + +func AdvanceStateUpToBlock(ctx context.Context, bc *core.BlockChain, state *state.StateDB, targetHeader *types.Header, lastAvailableHeader *types.Header, logFunc StateBuildingLogFunction) (*state.StateDB, error) { + returnedBlockNumber := targetHeader.Number.Uint64() + blockToRecreate := lastAvailableHeader.Number.Uint64() + 1 + prevHash := lastAvailableHeader.Hash() + for ctx.Err() == nil { + state, block, err := AdvanceStateByBlock(ctx, bc, state, targetHeader, blockToRecreate, prevHash, logFunc) + if err != nil { + return nil, err + } + prevHash = block.Hash() + if blockToRecreate >= returnedBlockNumber { + if block.Hash() != targetHeader.Hash() { + return nil, fmt.Errorf("blockHash doesn't match when recreating number: %d expected: %v got: %v", blockToRecreate, targetHeader.Hash(), block.Hash()) + } + return state, nil + } + blockToRecreate++ + } + return nil, ctx.Err() +} From af4fb22f472ef9242b69a8bf1a17ac3ff0ebb661 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 23 Feb 2024 11:11:05 +0000 Subject: [PATCH 17/21] cleanup debug log --- arbitrum/apibackend.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 4dd7bbec6c..ac5cce70da 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -7,7 +7,6 @@ import ( "math/big" "strconv" "strings" - "sync/atomic" "time" "github.com/ethereum/go-ethereum" @@ -46,8 +45,6 @@ type APIBackend struct { fallbackClient types.FallbackClient sync SyncProgressBackend - - recreatedStateFinalizers atomic.Int64 } type timeoutFallbackClient struct { @@ -518,14 +515,9 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream recreatedStatesCounter.Inc(1) - a.recreatedStateFinalizers.Add(1) statedb.SetRelease(func() { - // TODO remove logs and counters - a.recreatedStateFinalizers.Add(-1) - log.Warn("Recreated state release called", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load()) release() }) - log.Warn("Recreated state release set", "recreatedStateFinalizers", a.recreatedStateFinalizers.Load()) return statedb, header, err } From a2c45fb6efef261bf73e87eac39f51ebb55be388 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Fri, 23 Feb 2024 11:15:03 +0000 Subject: [PATCH 18/21] don't panic if StateDB.SetRelease is called more then once --- core/state/statedb.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index d771609ba3..4763f3c831 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -156,8 +156,7 @@ func (s *StateDB) SetRelease(release func()) { if s.release == nil { s.release = release } else { - // TODO - panic("StateDB.SetRelease called more then once") + log.Error("StateDB.SetRelease called more then once, may cause memory leak") } } From b432ef83821041f713842ac2b93f39c0be504959 Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Wed, 6 Mar 2024 12:54:24 +0000 Subject: [PATCH 19/21] bring back working finalizers --- arbitrum/apibackend.go | 22 +++++++++------ core/state/statedb.go | 51 +++++++++++----------------------- core/state/statedb_arbitrum.go | 12 +++++++- graphql/graphql.go | 3 -- internal/ethapi/api.go | 9 ------ 5 files changed, 40 insertions(+), 57 deletions(-) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index ac5cce70da..6000679582 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -36,8 +36,10 @@ import ( ) var ( - liveStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live", nil) - recreatedStatesCounter = metrics.NewRegisteredCounter("arb/apibackend/states/recreated", nil) + liveStatesReferencedCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/referenced", nil) + liveStatesDereferencedCounter = metrics.NewRegisteredCounter("arb/apibackend/states/live/dereferenced", nil) + recreatedStatesReferencedCounter = metrics.NewRegisteredCounter("arb/apibackend/states/recreated/referenced", nil) + recreatedStatesDereferencedCounter = metrics.NewRegisteredCounter("arb/apibackend/states/recreated/dereferenced", nil) ) type APIBackend struct { @@ -242,7 +244,6 @@ func (a *APIBackend) FeeHistory( return common.Big0, nil, nil, nil, err } speedLimit, err := core.GetArbOSSpeedLimitPerSecond(state) - state.Release() if err != nil { return common.Big0, nil, nil, nil, err } @@ -472,9 +473,10 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } liveState, liveStateRelease, err := stateFor(bc.StateCache(), bc.Snapshots())(header) if err == nil { - liveStatesCounter.Inc(1) - liveState.SetRelease(func() { + liveStatesReferencedCounter.Inc(1) + liveState.SetArbFinalizer(func(*state.ArbitrumExtraData) { liveStateRelease() + liveStatesDereferencedCounter.Inc(1) }) return liveState, header, nil } @@ -491,9 +493,10 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types } // make sure that we haven't found the state in diskdb if lastHeader == header { - liveStatesCounter.Inc(1) - lastState.SetRelease(func() { + liveStatesReferencedCounter.Inc(1) + lastState.SetArbFinalizer(func(*state.ArbitrumExtraData) { lastStateRelease() + liveStatesDereferencedCounter.Inc(1) }) return lastState, header, nil } @@ -514,9 +517,10 @@ func (a *APIBackend) stateAndHeaderFromHeader(ctx context.Context, header *types return nil, nil, fmt.Errorf("failed to recreate state: %w", err) } // we are setting finalizer instead of returning a StateReleaseFunc to avoid changing ethapi.Backend interface to minimize diff to upstream - recreatedStatesCounter.Inc(1) - statedb.SetRelease(func() { + recreatedStatesReferencedCounter.Inc(1) + statedb.SetArbFinalizer(func(*state.ArbitrumExtraData) { release() + recreatedStatesDereferencedCounter.Inc(1) }) return statedb, header, err } diff --git a/core/state/statedb.go b/core/state/statedb.go index 4763f3c831..3c65a61b05 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -62,8 +62,7 @@ type revision struct { // must be created with new root and updated database for accessing post- // commit states. type StateDB struct { - // Arbitrum: track the total balance change across all accounts - unexpectedBalanceDelta *big.Int + arbExtraData *ArbitrumExtraData // must be a pointer - can't be a part of StateDB allocation, otherwise its finalizer might not get called db Database prefetcher *triePrefetcher @@ -145,28 +144,9 @@ type StateDB struct { // Testing hooks onCommit func(states *triestate.Set) // Hook invoked when commit is performed - // arbitrum: cleanup hook - release func() - released bool - deterministic bool } -func (s *StateDB) SetRelease(release func()) { - if s.release == nil { - s.release = release - } else { - log.Error("StateDB.SetRelease called more then once, may cause memory leak") - } -} - -func (s *StateDB) Release() { - if s.release != nil && !s.released { - s.release() - s.released = true - } -} - // New creates a new state from a given trie. func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) { tr, err := db.OpenTrie(root) @@ -174,7 +154,9 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) return nil, err } sdb := &StateDB{ - unexpectedBalanceDelta: new(big.Int), + arbExtraData: &ArbitrumExtraData{ + unexpectedBalanceDelta: new(big.Int), + }, db: db, trie: tr, @@ -194,9 +176,6 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) accessList: newAccessList(), transientStorage: newTransientStorage(), hasher: crypto.NewKeccakState(), - - release: nil, - released: false, } if sdb.snaps != nil { sdb.snap = sdb.snaps.Snapshot(root) @@ -417,7 +396,7 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) { stateObject := s.GetOrNewStateObject(addr) if stateObject != nil { - s.unexpectedBalanceDelta.Add(s.unexpectedBalanceDelta, amount) + s.arbExtraData.unexpectedBalanceDelta.Add(s.arbExtraData.unexpectedBalanceDelta, amount) stateObject.AddBalance(amount) } } @@ -426,7 +405,7 @@ func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) { func (s *StateDB) SubBalance(addr common.Address, amount *big.Int) { stateObject := s.GetOrNewStateObject(addr) if stateObject != nil { - s.unexpectedBalanceDelta.Sub(s.unexpectedBalanceDelta, amount) + s.arbExtraData.unexpectedBalanceDelta.Sub(s.arbExtraData.unexpectedBalanceDelta, amount) stateObject.SubBalance(amount) } } @@ -438,8 +417,8 @@ func (s *StateDB) SetBalance(addr common.Address, amount *big.Int) { amount = big.NewInt(0) } prevBalance := stateObject.Balance() - s.unexpectedBalanceDelta.Add(s.unexpectedBalanceDelta, amount) - s.unexpectedBalanceDelta.Sub(s.unexpectedBalanceDelta, prevBalance) + s.arbExtraData.unexpectedBalanceDelta.Add(s.arbExtraData.unexpectedBalanceDelta, amount) + s.arbExtraData.unexpectedBalanceDelta.Sub(s.arbExtraData.unexpectedBalanceDelta, prevBalance) stateObject.SetBalance(amount) } } @@ -448,7 +427,7 @@ func (s *StateDB) ExpectBalanceBurn(amount *big.Int) { if amount.Sign() < 0 { panic(fmt.Sprintf("ExpectBalanceBurn called with negative amount %v", amount)) } - s.unexpectedBalanceDelta.Add(s.unexpectedBalanceDelta, amount) + s.arbExtraData.unexpectedBalanceDelta.Add(s.arbExtraData.unexpectedBalanceDelta, amount) } func (s *StateDB) SetNonce(addr common.Address, nonce uint64) { @@ -510,7 +489,7 @@ func (s *StateDB) SelfDestruct(addr common.Address) { }) stateObject.markSelfdestructed() - s.unexpectedBalanceDelta.Sub(s.unexpectedBalanceDelta, stateObject.data.Balance) + s.arbExtraData.unexpectedBalanceDelta.Sub(s.arbExtraData.unexpectedBalanceDelta, stateObject.data.Balance) stateObject.data.Balance = new(big.Int) } @@ -748,7 +727,9 @@ func (s *StateDB) CreateAccount(addr common.Address) { func (s *StateDB) Copy() *StateDB { // Copy all the basic fields, initialize the memory ones state := &StateDB{ - unexpectedBalanceDelta: new(big.Int).Set(s.unexpectedBalanceDelta), + arbExtraData: &ArbitrumExtraData{ + unexpectedBalanceDelta: new(big.Int).Set(s.arbExtraData.unexpectedBalanceDelta), + }, db: s.db, trie: s.db.CopyTrie(s.trie), @@ -853,7 +834,7 @@ func (s *StateDB) Copy() *StateDB { func (s *StateDB) Snapshot() int { id := s.nextRevisionId s.nextRevisionId++ - s.validRevisions = append(s.validRevisions, revision{id, s.journal.length(), new(big.Int).Set(s.unexpectedBalanceDelta)}) + s.validRevisions = append(s.validRevisions, revision{id, s.journal.length(), new(big.Int).Set(s.arbExtraData.unexpectedBalanceDelta)}) return id } @@ -868,7 +849,7 @@ func (s *StateDB) RevertToSnapshot(revid int) { } revision := s.validRevisions[idx] snapshot := revision.journalIndex - s.unexpectedBalanceDelta = new(big.Int).Set(revision.unexpectedBalanceDelta) + s.arbExtraData.unexpectedBalanceDelta = new(big.Int).Set(revision.unexpectedBalanceDelta) // Replay the journal to undo changes and remove invalidated snapshots s.journal.revert(s, snapshot) @@ -1344,7 +1325,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er s.snap = nil } - s.unexpectedBalanceDelta.Set(new(big.Int)) + s.arbExtraData.unexpectedBalanceDelta.Set(new(big.Int)) if root == (common.Hash{}) { root = types.EmptyRootHash diff --git a/core/state/statedb_arbitrum.go b/core/state/statedb_arbitrum.go index a77e4dd3ae..ce4b19b7ae 100644 --- a/core/state/statedb_arbitrum.go +++ b/core/state/statedb_arbitrum.go @@ -19,6 +19,7 @@ package state import ( "math/big" + "runtime" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -26,13 +27,22 @@ import ( "github.com/ethereum/go-ethereum/trie" ) +type ArbitrumExtraData struct { + // track the total balance change across all accounts + unexpectedBalanceDelta *big.Int +} + +func (s *StateDB) SetArbFinalizer(f func(*ArbitrumExtraData)) { + runtime.SetFinalizer(s.arbExtraData, f) +} + func (s *StateDB) GetCurrentTxLogs() []*types.Log { return s.logs[s.thash] } // GetUnexpectedBalanceDelta returns the total unexpected change in balances since the last commit to the database. func (s *StateDB) GetUnexpectedBalanceDelta() *big.Int { - return new(big.Int).Set(s.unexpectedBalanceDelta) + return new(big.Int).Set(s.arbExtraData.unexpectedBalanceDelta) } func (s *StateDB) GetSelfDestructs() []common.Address { diff --git a/graphql/graphql.go b/graphql/graphql.go index 86ef409110..ae4e5314d4 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -88,9 +88,6 @@ type Account struct { // getState fetches the StateDB object for an account. func (a *Account) getState(ctx context.Context) (*state.StateDB, error) { state, _, err := a.r.backend.StateAndHeaderByNumberOrHash(ctx, a.blockNrOrHash) - if state != nil && err == nil { - defer state.Release() - } return state, err } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index af085483c2..2d3fe10d11 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -630,7 +630,6 @@ func (s *BlockChainAPI) GetBalance(ctx context.Context, address common.Address, } return nil, err } - defer state.Release() return (*hexutil.Big)(state.GetBalance(address)), state.Error() } @@ -687,7 +686,6 @@ func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, st if state == nil || err != nil { return nil, err } - defer state.Release() if storageRoot := state.GetStorageRoot(address); storageRoot != types.EmptyRootHash && storageRoot != (common.Hash{}) { id := trie.StorageTrieID(header.Root, crypto.Keccak256Hash(address.Bytes()), storageRoot) tr, err := trie.NewStateTrie(id, state.Database().TrieDB()) @@ -886,7 +884,6 @@ func (s *BlockChainAPI) GetCode(ctx context.Context, address common.Address, blo } return nil, err } - defer state.Release() code := state.GetCode(address) return code, state.Error() } @@ -908,7 +905,6 @@ func (s *BlockChainAPI) GetStorageAt(ctx context.Context, address common.Address } return nil, err } - defer state.Release() res := state.GetState(address, key) return res[:], state.Error() } @@ -1200,7 +1196,6 @@ func DoCall(ctx context.Context, b Backend, args TransactionArgs, blockNrOrHash if state == nil || err != nil { return nil, err } - defer state.Release() header = updateHeaderForPendingBlocks(blockNrOrHash, header) return doCall(ctx, b, args, state, header, overrides, blockOverrides, timeout, globalGasCap, runMode) @@ -1330,7 +1325,6 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr if state == nil || err != nil { return 0, err } - defer state.Release() if err := overrides.Apply(state); err != nil { return 0, err } @@ -1367,7 +1361,6 @@ func DoEstimateGas(ctx context.Context, b Backend, args TransactionArgs, blockNr if state == nil || err != nil { return 0, err } - defer state.Release() gasCap, err = args.L2OnlyGasCap(gasCap, header, state, core.MessageGasEstimationMode) if err != nil { return 0, err @@ -1823,7 +1816,6 @@ func AccessList(ctx context.Context, b Backend, blockNrOrHash rpc.BlockNumberOrH if db == nil || err != nil { return nil, 0, nil, err } - defer db.Release() // If the gas amount is not set, default to RPC gas cap. if args.Gas == nil { tmp := hexutil.Uint64(b.RPCGasCap()) @@ -1963,7 +1955,6 @@ func (s *TransactionAPI) GetTransactionCount(ctx context.Context, address common } return nil, err } - defer state.Release() nonce := state.GetNonce(address) return (*hexutil.Uint64)(&nonce), state.Error() } From 57fcba9a2212c137eb17dfcd05308e3922a352ea Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Tue, 12 Mar 2024 21:57:10 +0000 Subject: [PATCH 20/21] add check for recent block in StateAndHeaderByNumberOrHash --- arbitrum/apibackend.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 08cb0a8d00..581ebf5e8a 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -532,6 +532,11 @@ func (a *APIBackend) StateAndHeaderByNumber(ctx context.Context, number rpc.Bloc func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*state.StateDB, *types.Header, error) { header, err := a.HeaderByNumberOrHash(ctx, blockNrOrHash) + hash, ishash := blockNrOrHash.Hash() + bc := a.BlockChain() + if ishash && header.Number.Cmp(bc.CurrentBlock().Number) > 0 && bc.GetCanonicalHash(header.Number.Uint64()) != hash { + return nil, nil, errors.New("requested block ahead of current block and the hash is not currently canonical") + } return a.stateAndHeaderFromHeader(ctx, header, err) } From 088149d73d7b39c844050e63f8a9c988ed8bdb2d Mon Sep 17 00:00:00 2001 From: Maciej Kulawik Date: Tue, 12 Mar 2024 22:03:16 +0000 Subject: [PATCH 21/21] add comment --- arbitrum/apibackend.go | 1 + 1 file changed, 1 insertion(+) diff --git a/arbitrum/apibackend.go b/arbitrum/apibackend.go index 581ebf5e8a..77af9239f3 100644 --- a/arbitrum/apibackend.go +++ b/arbitrum/apibackend.go @@ -534,6 +534,7 @@ func (a *APIBackend) StateAndHeaderByNumberOrHash(ctx context.Context, blockNrOr header, err := a.HeaderByNumberOrHash(ctx, blockNrOrHash) hash, ishash := blockNrOrHash.Hash() bc := a.BlockChain() + // check if we are not trying to get recent state that is not yet triedb referenced or committed in Blockchain.writeBlockWithState if ishash && header.Number.Cmp(bc.CurrentBlock().Number) > 0 && bc.GetCanonicalHash(header.Number.Uint64()) != hash { return nil, nil, errors.New("requested block ahead of current block and the hash is not currently canonical") }