From 6b1f676c60e4566141b80fcafd456fa5c2d38d81 Mon Sep 17 00:00:00 2001 From: Roy Crihfield Date: Wed, 19 Jul 2023 14:35:54 +0800 Subject: [PATCH] Builder clean up - Remove unused bits - simplify and normalize test data - Renames for clarity --- builder.go | 144 +++++++++++++--------------- builder_test.go | 12 +-- config.go | 5 +- indexer/database/metrics/metrics.go | 15 ++- indexer/mocks/test_data.go | 4 +- mainnet_tests/builder_test.go | 2 +- service.go | 8 +- test_helpers/builder.go | 55 +++++------ test_helpers/mocks/builder.go | 2 +- 9 files changed, 115 insertions(+), 132 deletions(-) diff --git a/builder.go b/builder.go index d7a89f0..456d5bb 100644 --- a/builder.go +++ b/builder.go @@ -44,18 +44,19 @@ import ( var ( emptyNode, _ = rlp.EncodeToBytes(&[]byte{}) emptyContractRoot = crypto.Keccak256Hash(emptyNode) - nullCodeHash = crypto.Keccak256Hash([]byte{}).Bytes() + nullCodeHash = crypto.Keccak256([]byte{}) zeroHash common.Hash ) // Builder interface exposes the method for building a state diff between two blocks type Builder interface { - BuildStateDiffObject(args Args, params Params) (sdtypes.StateObject, error) - WriteStateDiffObject(args Args, params Params, output sdtypes.StateNodeSink, ipldOutput sdtypes.IPLDSink) error + BuildStateDiffObject(Args, Params) (sdtypes.StateObject, error) + WriteStateDiff(Args, Params, sdtypes.StateNodeSink, sdtypes.IPLDSink) error } type StateDiffBuilder struct { - StateCache adapt.StateView + // state cache is safe for concurrent reads + stateCache adapt.StateView } type IterPair struct { @@ -84,7 +85,7 @@ func IPLDMappingAppender(iplds *[]sdtypes.IPLD) sdtypes.IPLDSink { // NewBuilder is used to create a statediff builder func NewBuilder(stateCache adapt.StateView) Builder { return &StateDiffBuilder{ - StateCache: stateCache, // state cache is safe for concurrent reads + stateCache: stateCache, } } @@ -93,7 +94,7 @@ func (sdb *StateDiffBuilder) BuildStateDiffObject(args Args, params Params) (sdt defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildStateDiffObjectTimer) var stateNodes []sdtypes.StateLeafNode var iplds []sdtypes.IPLD - err := sdb.WriteStateDiffObject(args, params, StateNodeAppender(&stateNodes), IPLDMappingAppender(&iplds)) + err := sdb.WriteStateDiff(args, params, StateNodeAppender(&stateNodes), IPLDMappingAppender(&iplds)) if err != nil { return sdtypes.StateObject{}, err } @@ -105,16 +106,16 @@ func (sdb *StateDiffBuilder) BuildStateDiffObject(args Args, params Params) (sdt }, nil } -// WriteStateDiffObject writes a statediff object to output sinks -func (sdb *StateDiffBuilder) WriteStateDiffObject(args Args, params Params, output sdtypes.StateNodeSink, - ipldOutput sdtypes.IPLDSink) error { - defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.WriteStateDiffObjectTimer) +// WriteStateDiff writes a statediff object to output sinks +func (sdb *StateDiffBuilder) WriteStateDiff(args Args, params Params, nodeSink sdtypes.StateNodeSink, + ipldSink sdtypes.IPLDSink) error { + defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.WriteStateDiffTimer) // Load tries for old and new states - oldTrie, err := sdb.StateCache.OpenTrie(args.OldStateRoot) + oldTrie, err := sdb.stateCache.OpenTrie(args.OldStateRoot) if err != nil { return fmt.Errorf("error creating trie for oldStateRoot: %w", err) } - newTrie, err := sdb.StateCache.OpenTrie(args.NewStateRoot) + newTrie, err := sdb.stateCache.OpenTrie(args.NewStateRoot) if err != nil { return fmt.Errorf("error creating trie for newStateRoot: %w", err) } @@ -134,28 +135,28 @@ func (sdb *StateDiffBuilder) WriteStateDiffObject(args Args, params Params, outp }, } - logger := log.New("hash", args.BlockHash.String(), "number", args.BlockNumber) - return sdb.BuildStateDiff(iterPairs, params, output, ipldOutput, logger, nil) + logger := log.New("hash", args.BlockHash, "number", args.BlockNumber) + return sdb.BuildStateDiff(iterPairs, params, nodeSink, ipldSink, logger) } func (sdb *StateDiffBuilder) BuildStateDiff(iterPairs []IterPair, params Params, - output sdtypes.StateNodeSink, ipldOutput sdtypes.IPLDSink, logger log.Logger, prefixPath []byte) error { + sinkNode sdtypes.StateNodeSink, sinkIpld sdtypes.IPLDSink, logger log.Logger) error { logger.Trace("statediff BEGIN BuildStateDiff") defer metrics.ReportAndUpdateDuration("statediff END BuildStateDiff", time.Now(), logger, metrics.IndexerMetrics.BuildStateDiffTimer) // collect a slice of all the nodes that were touched and exist at B (B-A) // a map of their leafkey to all the accounts that were touched and exist at B // and a slice of all the paths for the nodes in both of the above sets diffAccountsAtB, err := sdb.createdAndUpdatedState( - iterPairs[0].Older, iterPairs[0].Newer, params.watchedAddressesLeafPaths, ipldOutput, logger, prefixPath) + iterPairs[0].Older, iterPairs[0].Newer, params.watchedAddressesLeafPaths, sinkIpld, logger) if err != nil { return fmt.Errorf("error collecting createdAndUpdatedNodes: %w", err) } // collect a slice of all the nodes that existed at a path in A that doesn't exist in B // a map of their leafkey to all the accounts that were touched and exist at A - diffAccountsAtA, err := sdb.deletedOrUpdatedState( + diffAccountsAtA, err := sdb.deletedState( iterPairs[1].Older, iterPairs[1].Newer, diffAccountsAtB, - params.watchedAddressesLeafPaths, output, logger, prefixPath) + params.watchedAddressesLeafPaths, sinkNode, logger) if err != nil { return fmt.Errorf("error collecting deletedOrUpdatedNodes: %w", err) } @@ -177,12 +178,12 @@ func (sdb *StateDiffBuilder) BuildStateDiff(iterPairs []IterPair, params Params, "duration", time.Since(t)) // build the diff nodes for the updated accounts using the mappings at both A and B as directed by the keys found as the intersection of the two - err = sdb.buildAccountUpdates(diffAccountsAtB, diffAccountsAtA, updatedKeys, output, ipldOutput, logger) + err = sdb.buildAccountUpdates(diffAccountsAtB, diffAccountsAtA, updatedKeys, sinkNode, sinkIpld, logger) if err != nil { return fmt.Errorf("error building diff for updated accounts: %w", err) } // build the diff nodes for created accounts - err = sdb.buildAccountCreations(diffAccountsAtB, output, ipldOutput, logger) + err = sdb.buildAccountCreations(diffAccountsAtB, sinkNode, sinkIpld, logger) if err != nil { return fmt.Errorf("error building diff for created accounts: %w", err) } @@ -194,7 +195,7 @@ func (sdb *StateDiffBuilder) BuildStateDiff(iterPairs []IterPair, params Params, // a mapping of their leafkeys to all the accounts that exist in a different state at B than A // and a slice of the paths for all of the nodes included in both func (sdb *StateDiffBuilder) createdAndUpdatedState(a, b trie.NodeIterator, - watchedAddressesLeafPaths [][]byte, output sdtypes.IPLDSink, logger log.Logger, prefixPath []byte) (sdtypes.AccountMap, error) { + watchedAddressesLeafPaths [][]byte, ipldSink sdtypes.IPLDSink, logger log.Logger) (sdtypes.AccountMap, error) { logger.Trace("statediff BEGIN createdAndUpdatedState") defer metrics.ReportAndUpdateDuration("statediff END createdAndUpdatedState", time.Now(), logger, metrics.IndexerMetrics.CreatedAndUpdatedStateTimer) diffAccountsAtB := make(sdtypes.AccountMap) @@ -246,7 +247,7 @@ func (sdb *StateDiffBuilder) createdAndUpdatedState(a, b trie.NodeIterator, continue } } - if err := output(sdtypes.IPLD{ + if err := ipldSink(sdtypes.IPLD{ CID: ipld.Keccak256ToCid(ipld.MEthStateTrie, it.Hash().Bytes()).String(), Content: nodeVal, }); err != nil { @@ -276,12 +277,12 @@ func (sdb *StateDiffBuilder) processStateValueNode(it trie.NodeIterator, parentB }, nil } -// deletedOrUpdatedState returns a slice of all the paths that are emptied at B +// deletedState returns a slice of all the paths that are emptied at B // and a mapping of their leafkeys to all the accounts that exist in a different state at A than B -func (sdb *StateDiffBuilder) deletedOrUpdatedState(a, b trie.NodeIterator, diffAccountsAtB sdtypes.AccountMap, - watchedAddressesLeafPaths [][]byte, output sdtypes.StateNodeSink, logger log.Logger, prefixPath []byte) (sdtypes.AccountMap, error) { - logger.Trace("statediff BEGIN deletedOrUpdatedState") - defer metrics.ReportAndUpdateDuration("statediff END deletedOrUpdatedState", time.Now(), logger, metrics.IndexerMetrics.DeletedOrUpdatedStateTimer) +func (sdb *StateDiffBuilder) deletedState(a, b trie.NodeIterator, diffAccountsAtB sdtypes.AccountMap, + watchedAddressesLeafPaths [][]byte, nodeSink sdtypes.StateNodeSink, logger log.Logger) (sdtypes.AccountMap, error) { + logger.Trace("statediff BEGIN deletedState") + defer metrics.ReportAndUpdateDuration("statediff END deletedState", time.Now(), logger, metrics.IndexerMetrics.DeletedStateTimer) diffAccountAtA := make(sdtypes.AccountMap) var prevBlob []byte @@ -321,7 +322,7 @@ func (sdb *StateDiffBuilder) deletedOrUpdatedState(a, b trie.NodeIterator, diffA return nil, fmt.Errorf("failed building storage diffs for removed state account with key %x\r\nerror: %w", leafKey, err) } diff.StorageDiff = storageDiff - if err := output(diff); err != nil { + if err := nodeSink(diff); err != nil { return nil, err } } @@ -338,7 +339,7 @@ func (sdb *StateDiffBuilder) deletedOrUpdatedState(a, b trie.NodeIterator, diffA // needs to be called before building account creations and deletions as this mutates // those account maps to remove the accounts which were updated func (sdb *StateDiffBuilder) buildAccountUpdates(creations, deletions sdtypes.AccountMap, updatedKeys []string, - output sdtypes.StateNodeSink, ipldOutput sdtypes.IPLDSink, logger log.Logger) error { + nodeSink sdtypes.StateNodeSink, ipldSink sdtypes.IPLDSink, logger log.Logger) error { logger.Trace("statediff BEGIN buildAccountUpdates", "creations", len(creations), "deletions", len(deletions), "updated", len(updatedKeys)) defer metrics.ReportAndUpdateDuration("statediff END buildAccountUpdates ", @@ -351,13 +352,13 @@ func (sdb *StateDiffBuilder) buildAccountUpdates(creations, deletions sdtypes.Ac if deletedAcc.Account != nil && createdAcc.Account != nil { err = sdb.buildStorageNodesIncremental( deletedAcc.Account.Root, createdAcc.Account.Root, - StorageNodeAppender(&storageDiff), ipldOutput, + StorageNodeAppender(&storageDiff), ipldSink, ) if err != nil { return fmt.Errorf("failed building incremental storage diffs for account with leafkey %x\r\nerror: %w", key, err) } } - if err = output(sdtypes.StateLeafNode{ + if err = nodeSink(sdtypes.StateLeafNode{ AccountWrapper: createdAcc, Removed: false, StorageDiff: storageDiff, @@ -373,8 +374,8 @@ func (sdb *StateDiffBuilder) buildAccountUpdates(creations, deletions sdtypes.Ac // buildAccountCreations returns the statediff node objects for all the accounts that exist at B but not at A // it also returns the code and codehash for created contract accounts -func (sdb *StateDiffBuilder) buildAccountCreations(accounts sdtypes.AccountMap, output sdtypes.StateNodeSink, - ipldOutput sdtypes.IPLDSink, logger log.Logger) error { +func (sdb *StateDiffBuilder) buildAccountCreations(accounts sdtypes.AccountMap, nodeSink sdtypes.StateNodeSink, + ipldSink sdtypes.IPLDSink, logger log.Logger) error { logger.Trace("statediff BEGIN buildAccountCreations") defer metrics.ReportAndUpdateDuration("statediff END buildAccountCreations", time.Now(), logger, metrics.IndexerMetrics.BuildAccountCreationsTimer) @@ -386,25 +387,25 @@ func (sdb *StateDiffBuilder) buildAccountCreations(accounts sdtypes.AccountMap, if !bytes.Equal(val.Account.CodeHash, nullCodeHash) { // For contract creations, any storage node contained is a diff storageDiff := make([]sdtypes.StorageLeafNode, 0) - err := sdb.buildStorageNodesEventual(val.Account.Root, StorageNodeAppender(&storageDiff), ipldOutput) + err := sdb.buildStorageNodesEventual(val.Account.Root, StorageNodeAppender(&storageDiff), ipldSink) if err != nil { return fmt.Errorf("failed building eventual storage diffs for node with leaf key %x\r\nerror: %w", val.LeafKey, err) } diff.StorageDiff = storageDiff // emit codehash => code mappings for contract codeHash := common.BytesToHash(val.Account.CodeHash) - code, err := sdb.StateCache.ContractCode(codeHash) + code, err := sdb.stateCache.ContractCode(codeHash) if err != nil { - return fmt.Errorf("failed to retrieve code for codehash %x\r\n error: %w", codeHash, err) + return fmt.Errorf("failed to retrieve code for codehash %s\r\n error: %w", codeHash, err) } - if err := ipldOutput(sdtypes.IPLD{ + if err := ipldSink(sdtypes.IPLD{ CID: ipld.Keccak256ToCid(ipld.RawBinary, codeHash.Bytes()).String(), Content: code, }); err != nil { return err } } - if err := output(diff); err != nil { + if err := nodeSink(diff); err != nil { return err } } @@ -414,38 +415,31 @@ func (sdb *StateDiffBuilder) buildAccountCreations(accounts sdtypes.AccountMap, // buildStorageNodesEventual builds the storage diff node objects for a created account // i.e. it returns all the storage nodes at this state, since there is no previous state -func (sdb *StateDiffBuilder) buildStorageNodesEventual(sr common.Hash, output sdtypes.StorageNodeSink, - ipldOutput sdtypes.IPLDSink) error { +func (sdb *StateDiffBuilder) buildStorageNodesEventual(sr common.Hash, storageSink sdtypes.StorageNodeSink, + ipldSink sdtypes.IPLDSink) error { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildStorageNodesEventualTimer) if sr == emptyContractRoot { return nil } - log.Debug("Storage root for eventual diff", "root", sr.String()) - sTrie, err := sdb.StateCache.OpenTrie(sr) + log.Debug("Storage root for eventual diff", "root", sr) + sTrie, err := sdb.stateCache.OpenTrie(sr) if err != nil { log.Info("error in build storage diff eventual", "error", err) return err } + it := sTrie.NodeIterator(make([]byte, 0)) - return sdb.buildStorageNodesFromTrie(it, output, ipldOutput) -} - -// buildStorageNodesFromTrie returns all the storage diff node objects in the provided node iterator -func (sdb *StateDiffBuilder) buildStorageNodesFromTrie(it trie.NodeIterator, output sdtypes.StorageNodeSink, - ipldOutput sdtypes.IPLDSink) error { - defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildStorageNodesFromTrieTimer) - var prevBlob []byte for it.Next(true) { if it.Leaf() { storageLeafNode := sdb.processStorageValueNode(it, prevBlob) - if err := output(storageLeafNode); err != nil { + if err := storageSink(storageLeafNode); err != nil { return err } } else { nodeVal := make([]byte, len(it.NodeBlob())) copy(nodeVal, it.NodeBlob()) - if err := ipldOutput(sdtypes.IPLD{ + if err := ipldSink(sdtypes.IPLD{ CID: ipld.Keccak256ToCid(ipld.MEthStorageTrie, it.Hash().Bytes()).String(), Content: nodeVal, }); err != nil { @@ -474,29 +468,29 @@ func (sdb *StateDiffBuilder) processStorageValueNode(it trie.NodeIterator, paren } // buildRemovedAccountStorageNodes builds the "removed" diffs for all the storage nodes for a destroyed account -func (sdb *StateDiffBuilder) buildRemovedAccountStorageNodes(sr common.Hash, output sdtypes.StorageNodeSink) error { +func (sdb *StateDiffBuilder) buildRemovedAccountStorageNodes(sr common.Hash, storageSink sdtypes.StorageNodeSink) error { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildRemovedAccountStorageNodesTimer) if sr == emptyContractRoot { return nil } - log.Debug("Storage root for removed diffs", "root", sr.String()) - sTrie, err := sdb.StateCache.OpenTrie(sr) + log.Debug("Storage root for removed diffs", "root", sr) + sTrie, err := sdb.stateCache.OpenTrie(sr) if err != nil { log.Info("error in build removed account storage diffs", "error", err) return err } - it := sTrie.NodeIterator(make([]byte, 0)) - return sdb.buildRemovedStorageNodesFromTrie(it, output) + it := sTrie.NodeIterator(nil) + return sdb.buildRemovedStorageNodesFromTrie(it, storageSink) } // buildRemovedStorageNodesFromTrie returns diffs for all the storage nodes in the provided node interator -func (sdb *StateDiffBuilder) buildRemovedStorageNodesFromTrie(it trie.NodeIterator, output sdtypes.StorageNodeSink) error { +func (sdb *StateDiffBuilder) buildRemovedStorageNodesFromTrie(it trie.NodeIterator, storageSink sdtypes.StorageNodeSink) error { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildRemovedStorageNodesFromTrieTimer) for it.Next(true) { if it.Leaf() { // only leaf values are indexed, don't need to demarcate removed intermediate nodes leafKey := make([]byte, len(it.LeafKey())) copy(leafKey, it.LeafKey()) - if err := output(sdtypes.StorageLeafNode{ + if err := storageSink(sdtypes.StorageLeafNode{ CID: shared.RemovedNodeStorageCID, Removed: true, LeafKey: leafKey, @@ -510,33 +504,33 @@ func (sdb *StateDiffBuilder) buildRemovedStorageNodesFromTrie(it trie.NodeIterat } // buildStorageNodesIncremental builds the storage diff node objects for all nodes that exist in a different state at B than A -func (sdb *StateDiffBuilder) buildStorageNodesIncremental(oldroot common.Hash, newroot common.Hash, output sdtypes.StorageNodeSink, - ipldOutput sdtypes.IPLDSink) error { +func (sdb *StateDiffBuilder) buildStorageNodesIncremental(oldroot common.Hash, newroot common.Hash, storageSink sdtypes.StorageNodeSink, + ipldSink sdtypes.IPLDSink) error { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.BuildStorageNodesIncrementalTimer) if newroot == oldroot { return nil } - log.Trace("Storage roots for incremental diff", "old", oldroot.String(), "new", newroot.String()) - oldTrie, err := sdb.StateCache.OpenTrie(oldroot) + log.Trace("Storage roots for incremental diff", "old", oldroot, "new", newroot) + oldTrie, err := sdb.stateCache.OpenTrie(oldroot) if err != nil { return err } - newTrie, err := sdb.StateCache.OpenTrie(newroot) + newTrie, err := sdb.stateCache.OpenTrie(newroot) if err != nil { return err } diffSlotsAtB, err := sdb.createdAndUpdatedStorage( - oldTrie.NodeIterator([]byte{}), newTrie.NodeIterator([]byte{}), output, ipldOutput) + oldTrie.NodeIterator(nil), newTrie.NodeIterator(nil), storageSink, ipldSink) if err != nil { return err } - return sdb.deletedOrUpdatedStorage(oldTrie.NodeIterator([]byte{}), newTrie.NodeIterator([]byte{}), - diffSlotsAtB, output) + return sdb.deletedOrUpdatedStorage(oldTrie.NodeIterator(nil), newTrie.NodeIterator(nil), + diffSlotsAtB, storageSink) } -func (sdb *StateDiffBuilder) createdAndUpdatedStorage(a, b trie.NodeIterator, output sdtypes.StorageNodeSink, - ipldOutput sdtypes.IPLDSink) (map[string]bool, error) { +func (sdb *StateDiffBuilder) createdAndUpdatedStorage(a, b trie.NodeIterator, storageSink sdtypes.StorageNodeSink, + ipldSink sdtypes.IPLDSink) (map[string]bool, error) { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.CreatedAndUpdatedStorageTimer) diffSlotsAtB := make(map[string]bool) @@ -545,7 +539,7 @@ func (sdb *StateDiffBuilder) createdAndUpdatedStorage(a, b trie.NodeIterator, ou for it.Next(true) { if it.Leaf() { storageLeafNode := sdb.processStorageValueNode(it, prevBlob) - if err := output(storageLeafNode); err != nil { + if err := storageSink(storageLeafNode); err != nil { return nil, err } diffSlotsAtB[hex.EncodeToString(storageLeafNode.LeafKey)] = true @@ -555,10 +549,8 @@ func (sdb *StateDiffBuilder) createdAndUpdatedStorage(a, b trie.NodeIterator, ou } nodeVal := make([]byte, len(it.NodeBlob())) copy(nodeVal, it.NodeBlob()) - nodeHash := make([]byte, len(it.Hash().Bytes())) - copy(nodeHash, it.Hash().Bytes()) - if err := ipldOutput(sdtypes.IPLD{ - CID: ipld.Keccak256ToCid(ipld.MEthStorageTrie, nodeHash).String(), + if err := ipldSink(sdtypes.IPLD{ + CID: ipld.Keccak256ToCid(ipld.MEthStorageTrie, it.Hash().Bytes()).String(), Content: nodeVal, }); err != nil { return nil, err @@ -569,7 +561,7 @@ func (sdb *StateDiffBuilder) createdAndUpdatedStorage(a, b trie.NodeIterator, ou return diffSlotsAtB, it.Error() } -func (sdb *StateDiffBuilder) deletedOrUpdatedStorage(a, b trie.NodeIterator, diffSlotsAtB map[string]bool, output sdtypes.StorageNodeSink) error { +func (sdb *StateDiffBuilder) deletedOrUpdatedStorage(a, b trie.NodeIterator, diffSlotsAtB map[string]bool, storageSink sdtypes.StorageNodeSink) error { defer metrics.UpdateDuration(time.Now(), metrics.IndexerMetrics.DeletedOrUpdatedStorageTimer) it, _ := trie.NewDifferenceIterator(b, a) for it.Next(true) { @@ -580,7 +572,7 @@ func (sdb *StateDiffBuilder) deletedOrUpdatedStorage(a, b trie.NodeIterator, dif // that means the storage slot was vacated // in that case, emit an empty "removed" diff storage node if _, ok := diffSlotsAtB[hex.EncodeToString(leafKey)]; !ok { - if err := output(sdtypes.StorageLeafNode{ + if err := storageSink(sdtypes.StorageLeafNode{ CID: shared.RemovedNodeStorageCID, Removed: true, LeafKey: leafKey, diff --git a/builder_test.go b/builder_test.go index b4bb573..bdce724 100644 --- a/builder_test.go +++ b/builder_test.go @@ -37,8 +37,8 @@ import ( var ( contractLeafKey []byte - emptyDiffs = make([]sdtypes.StateLeafNode, 0) - emptyStorage = make([]sdtypes.StorageLeafNode, 0) + emptyDiffs []sdtypes.StateLeafNode + emptyStorage []sdtypes.StorageLeafNode block0, block1, block2, block3, block4, block5, block6 *types.Block builder statediff.Builder minerAddress = common.HexToAddress("0x0") @@ -2399,10 +2399,9 @@ func TestBuilderWithInternalizedLeafNodeAndWatchedAddress(t *testing.T) { &sdtypes.StateObject{ BlockNumber: block0.Number(), BlockHash: block0.Hash(), - Nodes: []sdtypes.StateLeafNode{}, - IPLDs: []sdtypes.IPLD{}, // there's some kind of weird behavior where if our root node is a leaf node - // even though it is along the path to the watched leaf (necessarily, as it is the root) it doesn't get included - // unconsequential, but kinda odd. + // there's some kind of weird behavior where if our root node is a leaf node even + // though it is along the path to the watched leaf (necessarily, as it is the root) + // it doesn't get included. unconsequential, but kinda odd. }, }, { @@ -2417,7 +2416,6 @@ func TestBuilderWithInternalizedLeafNodeAndWatchedAddress(t *testing.T) { &sdtypes.StateObject{ BlockNumber: block1.Number(), BlockHash: block1.Hash(), - Nodes: []sdtypes.StateLeafNode{}, IPLDs: []sdtypes.IPLD{ { CID: ipld.Keccak256ToCid(ipld.MEthStateTrie, crypto.Keccak256(block1bBranchRootNode)).String(), diff --git a/config.go b/config.go index 8f11ece..4b8246a 100644 --- a/config.go +++ b/config.go @@ -99,6 +99,7 @@ func (p *ParamsWithMutex) CopyParams() Params { // Args bundles the arguments for the state diff builder type Args struct { - OldStateRoot, NewStateRoot, BlockHash common.Hash - BlockNumber *big.Int + OldStateRoot, NewStateRoot common.Hash + BlockHash common.Hash + BlockNumber *big.Int } diff --git a/indexer/database/metrics/metrics.go b/indexer/database/metrics/metrics.go index f905d89..34fbaba 100644 --- a/indexer/database/metrics/metrics.go +++ b/indexer/database/metrics/metrics.go @@ -74,7 +74,7 @@ type IndexerMetricsHandles struct { // Fine-grained code timers BuildStateDiffTimer metrics.Timer CreatedAndUpdatedStateTimer metrics.Timer - DeletedOrUpdatedStateTimer metrics.Timer + DeletedStateTimer metrics.Timer BuildAccountUpdatesTimer metrics.Timer BuildAccountCreationsTimer metrics.Timer ResolveNodeTimer metrics.Timer @@ -88,9 +88,8 @@ type IndexerMetricsHandles struct { CreatedAndUpdatedStorageTimer metrics.Timer BuildStorageNodesIncrementalTimer metrics.Timer BuildStateDiffObjectTimer metrics.Timer - WriteStateDiffObjectTimer metrics.Timer + WriteStateDiffTimer metrics.Timer BuildStorageNodesEventualTimer metrics.Timer - BuildStorageNodesFromTrieTimer metrics.Timer BuildRemovedAccountStorageNodesTimer metrics.Timer BuildRemovedStorageNodesFromTrieTimer metrics.Timer IsWatchedAddressTimer metrics.Timer @@ -111,7 +110,7 @@ func RegisterIndexerMetrics(reg metrics.Registry) IndexerMetricsHandles { StateStoreCodeProcessingTimer: metrics.NewTimer(), BuildStateDiffTimer: metrics.NewTimer(), CreatedAndUpdatedStateTimer: metrics.NewTimer(), - DeletedOrUpdatedStateTimer: metrics.NewTimer(), + DeletedStateTimer: metrics.NewTimer(), BuildAccountUpdatesTimer: metrics.NewTimer(), BuildAccountCreationsTimer: metrics.NewTimer(), ResolveNodeTimer: metrics.NewTimer(), @@ -125,9 +124,8 @@ func RegisterIndexerMetrics(reg metrics.Registry) IndexerMetricsHandles { CreatedAndUpdatedStorageTimer: metrics.NewTimer(), BuildStorageNodesIncrementalTimer: metrics.NewTimer(), BuildStateDiffObjectTimer: metrics.NewTimer(), - WriteStateDiffObjectTimer: metrics.NewTimer(), + WriteStateDiffTimer: metrics.NewTimer(), BuildStorageNodesEventualTimer: metrics.NewTimer(), - BuildStorageNodesFromTrieTimer: metrics.NewTimer(), BuildRemovedAccountStorageNodesTimer: metrics.NewTimer(), BuildRemovedStorageNodesFromTrieTimer: metrics.NewTimer(), IsWatchedAddressTimer: metrics.NewTimer(), @@ -146,7 +144,7 @@ func RegisterIndexerMetrics(reg metrics.Registry) IndexerMetricsHandles { reg.Register(metricName(subsys, "t_state_store_code_processing"), ctx.StateStoreCodeProcessingTimer) reg.Register(metricName(subsys, "t_build_statediff"), ctx.BuildStateDiffTimer) reg.Register(metricName(subsys, "t_created_and_update_state"), ctx.CreatedAndUpdatedStateTimer) - reg.Register(metricName(subsys, "t_deleted_or_updated_state"), ctx.DeletedOrUpdatedStateTimer) + reg.Register(metricName(subsys, "t_deleted_or_updated_state"), ctx.DeletedStateTimer) reg.Register(metricName(subsys, "t_build_account_updates"), ctx.BuildAccountUpdatesTimer) reg.Register(metricName(subsys, "t_build_account_creations"), ctx.BuildAccountCreationsTimer) reg.Register(metricName(subsys, "t_resolve_node"), ctx.ResolveNodeTimer) @@ -160,10 +158,9 @@ func RegisterIndexerMetrics(reg metrics.Registry) IndexerMetricsHandles { reg.Register(metricName(subsys, "t_deleted_or_updated_storage"), ctx.DeletedOrUpdatedStorageTimer) reg.Register(metricName(subsys, "t_build_storage_nodes_incremental"), ctx.BuildStorageNodesIncrementalTimer) reg.Register(metricName(subsys, "t_build_statediff_object"), ctx.BuildStateDiffObjectTimer) - reg.Register(metricName(subsys, "t_write_statediff_object"), ctx.WriteStateDiffObjectTimer) + reg.Register(metricName(subsys, "t_write_statediff_object"), ctx.WriteStateDiffTimer) reg.Register(metricName(subsys, "t_created_and_updated_state"), ctx.CreatedAndUpdatedStateTimer) reg.Register(metricName(subsys, "t_build_storage_nodes_eventual"), ctx.BuildStorageNodesEventualTimer) - reg.Register(metricName(subsys, "t_build_storage_nodes_from_trie"), ctx.BuildStorageNodesFromTrieTimer) reg.Register(metricName(subsys, "t_build_removed_accounts_storage_nodes"), ctx.BuildRemovedAccountStorageNodesTimer) reg.Register(metricName(subsys, "t_build_removed_storage_nodes_from_trie"), ctx.BuildRemovedStorageNodesFromTrieTimer) reg.Register(metricName(subsys, "t_is_watched_address"), ctx.IsWatchedAddressTimer) diff --git a/indexer/mocks/test_data.go b/indexer/mocks/test_data.go index cefc205..07f6e59 100644 --- a/indexer/mocks/test_data.go +++ b/indexer/mocks/test_data.go @@ -227,7 +227,7 @@ var ( CID: AccountLeafNodeCID, }, Removed: false, - StorageDiff: []sdtypes.StorageLeafNode{}, + StorageDiff: nil, }, { AccountWrapper: sdtypes.AccountWrapper{ @@ -236,7 +236,7 @@ var ( CID: shared.RemovedNodeStateCID, }, Removed: true, - StorageDiff: []sdtypes.StorageLeafNode{}, + StorageDiff: nil, }, { AccountWrapper: sdtypes.AccountWrapper{ diff --git a/mainnet_tests/builder_test.go b/mainnet_tests/builder_test.go index e7e96f3..f31ca90 100644 --- a/mainnet_tests/builder_test.go +++ b/mainnet_tests/builder_test.go @@ -51,7 +51,7 @@ var ( block1CoinbaseAddr, block2CoinbaseAddr, block3CoinbaseAddr common.Address block1CoinbaseHash, block2CoinbaseHash, block3CoinbaseHash common.Hash builder statediff.Builder - emptyStorage = make([]sdtypes.StorageLeafNode, 0) + emptyStorage []sdtypes.StorageLeafNode // block 1 data block1CoinbaseAccount = &types.StateAccount{ diff --git a/service.go b/service.go index 4b89fd9..099e0a9 100644 --- a/service.go +++ b/service.go @@ -819,23 +819,23 @@ func (sds *Service) writeStateDiff(block *types.Block, parentRoot common.Hash, p return err } - output := func(node types2.StateLeafNode) error { + nodeSink := func(node types2.StateLeafNode) error { defer metrics.ReportAndUpdateDuration("statediff output", time.Now(), log, metrics.IndexerMetrics.OutputTimer) return sds.indexer.PushStateNode(tx, node, block.Hash().String()) } - ipldOutput := func(c types2.IPLD) error { + ipldSink := func(c types2.IPLD) error { defer metrics.ReportAndUpdateDuration("statediff ipldOutput", time.Now(), log, metrics.IndexerMetrics.IPLDOutputTimer) return sds.indexer.PushIPLD(tx, c) } - err = sds.Builder.WriteStateDiffObject(Args{ + err = sds.Builder.WriteStateDiff(Args{ NewStateRoot: block.Root(), OldStateRoot: parentRoot, BlockHash: block.Hash(), BlockNumber: block.Number(), - }, params, output, ipldOutput) + }, params, nodeSink, ipldSink) // TODO this anti-pattern needs to be sorted out eventually if err = tx.Submit(err); err != nil { diff --git a/test_helpers/builder.go b/test_helpers/builder.go index 66a11d4..12be63b 100644 --- a/test_helpers/builder.go +++ b/test_helpers/builder.go @@ -2,15 +2,13 @@ package test_helpers import ( "bytes" - "encoding/json" "sort" "testing" - "github.com/cerc-io/plugeth-statediff" + statediff "github.com/cerc-io/plugeth-statediff" sdtypes "github.com/cerc-io/plugeth-statediff/types" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" - "github.com/ethereum/go-ethereum/rlp" "github.com/stretchr/testify/require" ) @@ -34,34 +32,10 @@ func RunBuilderTests( if err != nil { t.Error(err) } - receivedStateDiffRlp, err := rlp.EncodeToBytes(&diff) - if err != nil { - t.Error(err) - } - expectedStateDiffRlp, err := rlp.EncodeToBytes(test.Expected) - if err != nil { - t.Error(err) - } - sort.Slice(receivedStateDiffRlp, func(i, j int) bool { - return receivedStateDiffRlp[i] < receivedStateDiffRlp[j] - }) - sort.Slice(expectedStateDiffRlp, func(i, j int) bool { - return expectedStateDiffRlp[i] < expectedStateDiffRlp[j] - }) - if !bytes.Equal(receivedStateDiffRlp, expectedStateDiffRlp) { - actualb, err := json.Marshal(diff) - require.NoError(t, err) - expectedb, err := json.Marshal(test.Expected) - require.NoError(t, err) - var expected, actual interface{} - err = json.Unmarshal(expectedb, &expected) - require.NoError(t, err) - err = json.Unmarshal(actualb, &actual) - require.NoError(t, err) - - require.Equal(t, expected, actual, test.Name) - } + normalize(test.Expected) + normalize(&diff) + require.Equal(t, *test.Expected, diff, test.Name) } // Let's also confirm that our root state nodes form the state root hash in the headers for block, node := range roots { @@ -69,3 +43,24 @@ func RunBuilderTests( "expected root does not match actual root", block.Number()) } } + +// Sorts contained state nodes, storage nodes, and IPLDs +func normalize(diff *sdtypes.StateObject) { + sort.Slice(diff.IPLDs, func(i, j int) bool { + return diff.IPLDs[i].CID < diff.IPLDs[j].CID + }) + sort.Slice(diff.Nodes, func(i, j int) bool { + return bytes.Compare( + diff.Nodes[i].AccountWrapper.LeafKey, + diff.Nodes[j].AccountWrapper.LeafKey, + ) < 0 + }) + for _, node := range diff.Nodes { + sort.Slice(node.StorageDiff, func(i, j int) bool { + return bytes.Compare( + node.StorageDiff[i].LeafKey, + node.StorageDiff[j].LeafKey, + ) < 0 + }) + } +} diff --git a/test_helpers/mocks/builder.go b/test_helpers/mocks/builder.go index c446a29..a3457b0 100644 --- a/test_helpers/mocks/builder.go +++ b/test_helpers/mocks/builder.go @@ -40,7 +40,7 @@ func (builder *Builder) BuildStateDiffObject(args statediff.Args, params statedi } // BuildStateDiffObject mock method -func (builder *Builder) WriteStateDiffObject(args statediff.Args, params statediff.Params, output sdtypes.StateNodeSink, iplds sdtypes.IPLDSink) error { +func (builder *Builder) WriteStateDiff(args statediff.Args, params statediff.Params, output sdtypes.StateNodeSink, iplds sdtypes.IPLDSink) error { builder.Args = args builder.Params = params