Add 9 bytes of metadata to FlatKV store#3163
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3163 +/- ##
==========================================
- Coverage 59.02% 58.71% -0.31%
==========================================
Files 2065 2106 +41
Lines 169349 173563 +4214
==========================================
+ Hits 99960 101912 +1952
- Misses 60630 62535 +1905
- Partials 8759 9116 +357
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| // Setup // | ||
| /////////// | ||
| s.phaseTimer.SetPhase("apply_change_sets_prepare") | ||
| s.pendingChangeSets = append(s.pendingChangeSets, changeSets...) |
There was a problem hiding this comment.
ApplyChangeSets now appends to pendingChangeSets before validation/processing succeeds, while Commit writes pendingChangeSets straight into the WAL. If one ApplyChangeSets fails and the caller later commits, the WAL can contain changes that were never reflected in the in-memory writes/LtHash, and catchup will replay bad data.
consider only commit to s.pendingChangeSets, s.accountWrites, etc. after the entire function succeeds
|
|
||
| // GetBlockHeightModified returns the block height at which the key was last modified. | ||
| // If not found, returns (-1, false, nil). | ||
| func (s *CommitStore) GetBlockHeightModified(key []byte) (int64, bool, error) { |
There was a problem hiding this comment.
For an EOA (zero code-hash), Get(codeHashKey) returns (nil, false, nil), correctly reporting not found.
But GetBlockHeightModified(codeHashKey) returns (blockHeight, true, nil) because it doesn't check for zero code-hash.
Inconsistent API semantics.
| } | ||
| return cd.GetBlockHeight(), true, nil | ||
| default: | ||
| return -1, false, fmt.Errorf("unsupported key type: %v", kind) |
There was a problem hiding this comment.
should we add EVMKeyLegacy switch support here? @yzang2019
|
file name spelling: |
| // Process incoming legacy changes into a form appropriate for hashing and insertion into the DB. | ||
| func processLegacyChanges( | ||
| rawChanges map[string][]byte, | ||
| blockHeight int64, |
There was a problem hiding this comment.
is the blockHeight param being used? and LegacyData has no block height field
3 participants
Describe your changes and provide context
In order to support BUD proofs, we need to add "block last modified" metadata to each value. Since we are about to do a migration, it's lower effort in the long run to add this metadata now instead of later.
Testing performed to validate your change
Unit tests