Refactor States To Allow for Single Cached Hasher #9922
Conversation
…geth-sharding into refactorFieldRoot
| if len(prevLeaves) != len(leaves) { | ||
| res, err := h.merkleizeWithCache(leaves, length, fieldName, hashFunc) | ||
| if err != nil { | ||
| return [32]byte{}, err | ||
| } | ||
| if h.rootsCache != nil { | ||
| leavesCache[fieldName] = leaves | ||
| } | ||
| return res, nil | ||
| } |
There was a problem hiding this comment.
A bit out of scope for a refactor
|
can we merge v3 state PRs first before we merge this? it'll be less disparity and prone to error |
| ) | ||
|
|
||
| var ( | ||
| leavesCache = make(map[string][][32]byte, params.BeaconConfig().BeaconStateMergeFieldCount) |
There was a problem hiding this comment.
Are we just always taking the highest field count here? if yes, could we please add a comment to clarify?
|
|
||
| // RootsArrayHashTreeRoot computes the Merkle root of arrays of 32-byte hashes, such as [64][32]byte | ||
| // according to the Simple Serialize specification of Ethereum. | ||
| func RootsArrayHashTreeRoot(vals [][]byte, length uint64, fieldName string) ([32]byte, error) { |
There was a problem hiding this comment.
This is only used in tests, can we delete this function?
There was a problem hiding this comment.
This is used in runtime as part of computeFieldRoots. This is why we continue using it, this should be removed later but I will need a bit more data to be sure on this.
| return [32]byte{}, errors.New("nil pending attestation") | ||
| } | ||
| // Marshal attestation to determine if it exists in the cache. | ||
| enc := PendingAttEncKey(att) |
There was a problem hiding this comment.
PendingAttEncKey no longer needs to be exported
| } | ||
| } | ||
|
|
||
| res, err := PendingAttRootWithHasher(hasher, att) |
There was a problem hiding this comment.
PendingAttRootWithHasher no longer needs to be exported
| if eth1Data == nil { | ||
| return [32]byte{}, errors.New("nil eth1 data") | ||
| } | ||
|
|
||
| enc := stateutil.Eth1DataEncKey(eth1Data) | ||
| enc := Eth1DataEncKey(eth1Data) |
There was a problem hiding this comment.
Eth1DataEncKey no longer needs to be exported
| @@ -59,15 +68,15 @@ func (h *stateRootHasher) validatorRoot(hasher ssz.HashFn, validator *ethpb.Vali | |||
| return [32]byte{}, errors.New("nil validator") | |||
| } | |||
|
|
|||
| enc := stateutil.ValidatorEncKey(validator) | |||
| enc := ValidatorEncKey(validator) | |||
There was a problem hiding this comment.
ValidatorEncKey no longer needs to be exported
| var NocachedHasher *stateRootHasher | ||
|
|
||
| // CachedHasher references a hasher that will utilize a roots cache. | ||
| var CachedHasher *stateRootHasher |
There was a problem hiding this comment.
One minor downside with this is we have to export these hashers to be shared with state pkgs
remove assigned ctx
This reverts commit 37bc407. # Conflicts: # beacon-chain/state/fieldtrie/field_trie_test.go # beacon-chain/state/stateutil/trie_helpers_test.go # beacon-chain/state/v1/BUILD.bazel # beacon-chain/state/v1/field_roots.go # beacon-chain/state/v1/getters_misc.go # beacon-chain/state/v1/state_trie.go # beacon-chain/state/v2/BUILD.bazel # beacon-chain/state/v2/field_roots.go # beacon-chain/state/v2/state_trie.go
)"" This reverts commit 928e05e. # Conflicts: # beacon-chain/state/v3/BUILD.bazel # beacon-chain/state/v3/field_roots.go # beacon-chain/state/v3/state_trie.go
What type of PR is this?
Refactor
What does this PR do? Why is it needed?
stateutilpackage so thatonly a single implementation of it exists. This reduces duplication as now it only has to be implemented once rather than multiple times for each state type.
Which issues(s) does this PR fix?
Fixes #
Other notes for review