From 9691ef80d7f532d3003161c4d2456d9eeb68db79 Mon Sep 17 00:00:00 2001 From: Rob Mulholand Date: Thu, 12 Sep 2019 14:01:26 -0500 Subject: [PATCH] Derive state diffs from Commit rather than Process - avoids looping through dirty state objects twice --- cmd/geth/retesteth.go | 4 ++-- core/blockchain.go | 26 +++++++++++++------------- core/blockchain_test.go | 2 +- core/chain_makers.go | 2 +- core/state/state_test.go | 2 +- core/state/statedb.go | 26 ++++++-------------------- core/state/statedb_test.go | 6 +++--- core/state/sync_test.go | 2 +- core/state_processor.go | 8 +++----- core/types.go | 3 +-- eth/api_tracer.go | 8 ++++---- miner/worker.go | 4 ++-- tests/state_test_util.go | 2 +- 13 files changed, 39 insertions(+), 56 deletions(-) diff --git a/cmd/geth/retesteth.go b/cmd/geth/retesteth.go index 9469c9f5f..bb2f40090 100644 --- a/cmd/geth/retesteth.go +++ b/cmd/geth/retesteth.go @@ -665,7 +665,7 @@ func (api *RetestethAPI) AccountRange(ctx context.Context, root = statedb.IntermediateRoot(vmenv.ChainConfig().IsEIP158(block.Number())) if idx == int(txIndex) { // This is to make sure root can be opened by OpenTrie - root, err = statedb.Commit(api.chainConfig.IsEIP158(block.Number())) + root, _, err = statedb.Commit(api.chainConfig.IsEIP158(block.Number())) if err != nil { return AccountRangeResult{}, err } @@ -778,7 +778,7 @@ func (api *RetestethAPI) StorageRangeAt(ctx context.Context, _ = statedb.IntermediateRoot(vmenv.ChainConfig().IsEIP158(block.Number())) if idx == int(txIndex) { // This is to make sure root can be opened by OpenTrie - _, err = statedb.Commit(vmenv.ChainConfig().IsEIP158(block.Number())) + _, _, err = statedb.Commit(vmenv.ChainConfig().IsEIP158(block.Number())) if err != nil { return StorageRangeResult{}, err } diff --git a/core/blockchain.go b/core/blockchain.go index 2594ba955..cd8fec010 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1253,7 +1253,7 @@ func (bc *BlockChain) writeKnownBlock(block *types.Block) error { } // WriteBlockWithState writes the block and all associated state to the database. -func (bc *BlockChain) WriteBlockWithState(block *types.Block, receipts []*types.Receipt, state *state.StateDB) (status WriteStatus, err error) { +func (bc *BlockChain) WriteBlockWithState(block *types.Block, receipts []*types.Receipt, state *state.StateDB) (status WriteStatus, stateDiffs map[common.Address]state.Account, err error) { bc.chainmu.Lock() defer bc.chainmu.Unlock() @@ -1262,14 +1262,14 @@ func (bc *BlockChain) WriteBlockWithState(block *types.Block, receipts []*types. // writeBlockWithState writes the block and all associated state to the database, // but is expects the chain mutex to be held. -func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.Receipt, state *state.StateDB) (status WriteStatus, err error) { +func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.Receipt, state *state.StateDB) (status WriteStatus, stateDiffs map[common.Address]state.Account, err error) { bc.wg.Add(1) defer bc.wg.Done() // Calculate the total difficulty of the block ptd := bc.GetTd(block.ParentHash(), block.NumberU64()-1) if ptd == nil { - return NonStatTy, consensus.ErrUnknownAncestor + return NonStatTy, stateDiffs, consensus.ErrUnknownAncestor } // Make sure no inconsistent state is leaked during insertion currentBlock := bc.CurrentBlock() @@ -1278,20 +1278,20 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. // Irrelevant of the canonical status, write the block itself to the database if err := bc.hc.WriteTd(block.Hash(), block.NumberU64(), externTd); err != nil { - return NonStatTy, err + return NonStatTy, stateDiffs, err } rawdb.WriteBlock(bc.db, block) - root, err := state.Commit(bc.chainConfig.IsEIP158(block.Number())) + root, modifiedAccounts, err := state.Commit(bc.chainConfig.IsEIP158(block.Number())) if err != nil { - return NonStatTy, err + return NonStatTy, stateDiffs, err } triedb := bc.stateCache.TrieDB() // If we're running an archive node, always flush if bc.cacheConfig.TrieDirtyDisabled { if err := triedb.Commit(root, false); err != nil { - return NonStatTy, err + return NonStatTy, modifiedAccounts, err } } else { // Full but not archive node, do proper garbage collection @@ -1367,7 +1367,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. // Reorganise the chain if the parent is not the head block if block.ParentHash() != currentBlock.Hash() { if err := bc.reorg(currentBlock, block); err != nil { - return NonStatTy, err + return NonStatTy, modifiedAccounts, err } } // Write the positional metadata for transaction/receipt lookups and preimages @@ -1379,7 +1379,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. status = SideStatTy } if err := batch.Write(); err != nil { - return NonStatTy, err + return NonStatTy, modifiedAccounts, err } // Set new head. @@ -1387,7 +1387,7 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types. bc.insert(block) } bc.futureBlocks.Remove(block.Hash()) - return status, nil + return status, modifiedAccounts, nil } // addFutureBlock checks if the block is within the max allowed window to get @@ -1623,7 +1623,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, [] } // Process block using the parent state as reference point substart := time.Now() - receipts, logs, processedStateDiffs, usedGas, err := bc.processor.Process(block, statedb, bc.vmConfig) + receipts, logs, usedGas, err := bc.processor.Process(block, statedb, bc.vmConfig) if err != nil { bc.reportBlock(block, receipts, err) atomic.StoreUint32(&followupInterrupt, 1) @@ -1658,7 +1658,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, [] // Write the block to the chain and get the status. substart = time.Now() - status, err := bc.writeBlockWithState(block, receipts, statedb) + status, committedStateDiffs, err := bc.writeBlockWithState(block, receipts, statedb) if err != nil { atomic.StoreUint32(&followupInterrupt, 1) return it.index, events, coalescedLogs, stateDiffs, err @@ -1680,7 +1680,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, [] "root", block.Root()) coalescedLogs = append(coalescedLogs, logs...) - stateDiffs = processedStateDiffs + stateDiffs = committedStateDiffs events = append(events, ChainEvent{block, block.Hash(), logs}) lastCanon = block diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 965e356cf..db624c4dc 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -147,7 +147,7 @@ func testBlockChainImport(chain types.Blocks, blockchain *BlockChain) error { if err != nil { return err } - receipts, _, _, usedGas, err := blockchain.processor.Process(block, statedb, vm.Config{}) + receipts, _, usedGas, err := blockchain.processor.Process(block, statedb, vm.Config{}) if err != nil { blockchain.reportBlock(block, receipts, err) return err diff --git a/core/chain_makers.go b/core/chain_makers.go index 17f404211..d1e9cf92d 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -216,7 +216,7 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse block, _ := b.engine.FinalizeAndAssemble(chainreader, b.header, statedb, b.txs, b.uncles, b.receipts) // Write state changes to db - root, err := statedb.Commit(config.IsEIP158(b.header.Number)) + root, _, err := statedb.Commit(config.IsEIP158(b.header.Number)) if err != nil { panic(fmt.Sprintf("state write error: %v", err)) } diff --git a/core/state/state_test.go b/core/state/state_test.go index d6ff714ee..849cef1f8 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -158,7 +158,7 @@ func TestSnapshot2(t *testing.T) { so0.deleted = false state.setStateObject(so0) - root, _ := state.Commit(false) + root, _, _ := state.Commit(false) state.Reset(root) // and one with deleted == true diff --git a/core/state/statedb.go b/core/state/statedb.go index 7efe1c162..d3231eac8 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -685,27 +685,11 @@ func (s *StateDB) clearJournalAndRefund() { s.refund = 0 } -func (s *StateDB) GetDirtyAccounts() map[common.Address]Account { - for addr := range s.journal.dirties { - s.stateObjectsDirty[addr] = struct{}{} - } - - results := make(map[common.Address]Account) - - for addr, stateObject := range s.stateObjects { - _, isDirty := s.stateObjectsDirty[addr] - if isDirty { - results[addr] = stateObject.data - } - } - - return results -} - // Commit writes the state to the underlying in-memory trie database. -func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) { +func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, modifiedAccounts map[common.Address]Account, err error) { defer s.clearJournalAndRefund() + modifiedAccounts = make(map[common.Address]Account) for addr := range s.journal.dirties { s.stateObjectsDirty[addr] = struct{}{} } @@ -716,6 +700,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) case stateObject.suicided || (isDirty && deleteEmptyObjects && stateObject.empty()): // If the object has been removed, don't bother syncing it // and just mark it for deletion in the trie. + modifiedAccounts[addr] = stateObject.data s.deleteStateObject(stateObject) case isDirty: // Write any contract code associated with the state object @@ -725,8 +710,9 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) } // Write any storage changes in the state object to its storage trie. if err := stateObject.CommitTrie(s.db); err != nil { - return common.Hash{}, err + return common.Hash{}, nil, err } + modifiedAccounts[addr] = stateObject.data // Update the object in the main account trie. s.updateStateObject(stateObject) } @@ -750,5 +736,5 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (root common.Hash, err error) } return nil }) - return root, err + return root, modifiedAccounts, err } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index bf073bc94..32608ded4 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -98,10 +98,10 @@ func TestIntermediateLeaks(t *testing.T) { } // Commit and cross check the databases. - if _, err := transState.Commit(false); err != nil { + if _, _, err := transState.Commit(false); err != nil { t.Fatalf("failed to commit transition state: %v", err) } - if _, err := finalState.Commit(false); err != nil { + if _, _, err := finalState.Commit(false); err != nil { t.Fatalf("failed to commit final state: %v", err) } it := finalDb.NewIterator() @@ -420,7 +420,7 @@ func (test *snapshotTest) checkEqual(state, checkstate *StateDB) error { func (s *StateSuite) TestTouchDelete(c *check.C) { s.state.GetOrNewStateObject(common.Address{}) - root, _ := s.state.Commit(false) + root, _, _ := s.state.Commit(false) s.state.Reset(root) snapshot := s.state.Snapshot() diff --git a/core/state/sync_test.go b/core/state/sync_test.go index de098dce0..ea2e1322d 100644 --- a/core/state/sync_test.go +++ b/core/state/sync_test.go @@ -62,7 +62,7 @@ func makeTestState() (Database, common.Hash, []*testAccount) { state.updateStateObject(obj) accounts = append(accounts, acc) } - root, _ := state.Commit(false) + root, _, _ := state.Commit(false) // Return the generated state return db, root, accounts diff --git a/core/state_processor.go b/core/state_processor.go index 18860eb2b..bed6a0730 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -53,7 +53,7 @@ func NewStateProcessor(config *params.ChainConfig, bc *BlockChain, engine consen // Process returns the receipts and logs accumulated during the process and // returns the amount of gas that was used in the process. If any of the // transactions failed to execute due to insufficient gas it will return an error. -func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg vm.Config) (types.Receipts, []*types.Log, map[common.Address]state.Account, uint64, error) { +func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg vm.Config) (types.Receipts, []*types.Log, uint64, error) { var ( receipts types.Receipts usedGas = new(uint64) @@ -70,7 +70,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg statedb.Prepare(tx.Hash(), block.Hash(), i) receipt, _, err := ApplyTransaction(p.config, p.bc, nil, gp, statedb, header, tx, usedGas, cfg) if err != nil { - return nil, nil, nil, 0, err + return nil, nil, 0, err } receipts = append(receipts, receipt) allLogs = append(allLogs, receipt.Logs...) @@ -78,9 +78,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg // Finalize the block, applying any consensus engine specific extras (e.g. block rewards) p.engine.Finalize(p.bc, header, statedb, block.Transactions(), block.Uncles()) - stateDiffs := statedb.GetDirtyAccounts() - - return receipts, allLogs, stateDiffs, *usedGas, nil + return receipts, allLogs, *usedGas, nil } // ApplyTransaction attempts to apply a transaction to the given state database diff --git a/core/types.go b/core/types.go index 50cd1bed6..4c5b74a49 100644 --- a/core/types.go +++ b/core/types.go @@ -17,7 +17,6 @@ package core import ( - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/state" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" @@ -48,5 +47,5 @@ type Processor interface { // Process processes the state changes according to the Ethereum rules by running // the transaction messages using the statedb and applying any rewards to both // the processor (coinbase) and any included uncles. - Process(block *types.Block, statedb *state.StateDB, cfg vm.Config) (types.Receipts, []*types.Log, map[common.Address]state.Account, uint64, error) + Process(block *types.Block, statedb *state.StateDB, cfg vm.Config) (types.Receipts, []*types.Log, uint64, error) } diff --git a/eth/api_tracer.go b/eth/api_tracer.go index a3bf2db06..05dc74dec 100644 --- a/eth/api_tracer.go +++ b/eth/api_tracer.go @@ -289,13 +289,13 @@ func (api *PrivateDebugAPI) traceChain(ctx context.Context, start, end *types.Bl traced += uint64(len(txs)) } // Generate the next state snapshot fast without tracing - _, _, _, _, err := api.eth.blockchain.Processor().Process(block, statedb, vm.Config{}) + _, _, _, err := api.eth.blockchain.Processor().Process(block, statedb, vm.Config{}) if err != nil { failed = err break } // Finalize the state so any modifications are written to the trie - root, err := statedb.Commit(api.eth.blockchain.Config().IsEIP158(block.Number())) + root, _, err := statedb.Commit(api.eth.blockchain.Config().IsEIP158(block.Number())) if err != nil { failed = err break @@ -676,12 +676,12 @@ func (api *PrivateDebugAPI) computeStateDB(block *types.Block, reexec uint64) (* if block = api.eth.blockchain.GetBlockByNumber(block.NumberU64() + 1); block == nil { return nil, fmt.Errorf("block #%d not found", block.NumberU64()+1) } - _, _, _, _, err := api.eth.blockchain.Processor().Process(block, statedb, vm.Config{}) + _, _, _, err := api.eth.blockchain.Processor().Process(block, statedb, vm.Config{}) if err != nil { return nil, fmt.Errorf("processing block %d failed: %v", block.NumberU64(), err) } // Finalize the state so any modifications are written to the trie - root, err := statedb.Commit(api.eth.blockchain.Config().IsEIP158(block.Number())) + root, _, err := statedb.Commit(api.eth.blockchain.Config().IsEIP158(block.Number())) if err != nil { return nil, err } diff --git a/miner/worker.go b/miner/worker.go index d44551916..5ba40aa9a 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -589,7 +589,7 @@ func (w *worker) resultLoop() { logs = append(logs, receipt.Logs...) } // Commit block and state to database. - stat, err := w.chain.WriteBlockWithState(block, receipts, task.state) + stat, stateDiffs, err := w.chain.WriteBlockWithState(block, receipts, task.state) if err != nil { log.Error("Failed writing block to chain", "err", err) continue @@ -608,7 +608,7 @@ func (w *worker) resultLoop() { case core.SideStatTy: events = append(events, core.ChainSideEvent{Block: block}) } - w.chain.PostChainEvents(events, logs, task.state.GetDirtyAccounts()) + w.chain.PostChainEvents(events, logs, stateDiffs) // Insert the block into the set of pending ones to resultLoop for confirmations w.unconfirmed.Insert(block.NumberU64(), block.Hash()) diff --git a/tests/state_test_util.go b/tests/state_test_util.go index 59ebcb6e1..8d8cf0be5 100644 --- a/tests/state_test_util.go +++ b/tests/state_test_util.go @@ -216,7 +216,7 @@ func MakePreState(db ethdb.Database, accounts core.GenesisAlloc) *state.StateDB } } // Commit and re-open to start with a clean state. - root, _ := statedb.Commit(false) + root, _, _ := statedb.Commit(false) statedb, _ = state.New(root, sdb) return statedb }