-
Notifications
You must be signed in to change notification settings - Fork 209
Cache EVM block proposal and defer storage writes to commit #8491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
holyfuchs
wants to merge
12
commits into
master
Choose a base branch
from
holyfuchs/6958-evm-optimize-block-formation
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
4312d4d
fvm/evm: cache and defer EVM block proposal persistence to transactio…
holyfuchs 5b7f841
fvm/evm: move BlockStore to fvm environment as EVMBlockStore
holyfuchs edf84f5
add chainID to TestBackend
holyfuchs 5048baf
fvm/evm: add regression test for dryRun followed by run in same trans…
holyfuchs 3896575
reduce block store interface size
janezpodhostnik cd645a8
Merge branch 'master' into holyfuchs/6958-evm-optimize-block-formation
holyfuchs a2e6bc9
add evm block store test case
zhangchiqing 9dff1a3
remove latest block proposal key on commit block proposal
zhangchiqing 3768a7e
Merge pull request #8546 from onflow/leo/suggestion-6958
holyfuchs 9d95174
fix lint
zhangchiqing d92c088
fix lint
zhangchiqing 98cdea4
write new LatestBlockProposal in ComimtBlockProposal
zhangchiqing File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,297 @@ | ||
| package environment | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "time" | ||
|
|
||
| gethCommon "github.com/ethereum/go-ethereum/common" | ||
|
|
||
| "github.com/onflow/flow-go/fvm/evm/types" | ||
| "github.com/onflow/flow-go/model/flow" | ||
| ) | ||
|
|
||
| // BlockStore stores the chain of blocks | ||
| type EVMBlockStore interface { | ||
| // LatestBlock returns the latest appended block | ||
| LatestBlock() (*types.Block, error) | ||
|
|
||
| // BlockHash returns the hash of the block at the given height | ||
| BlockHash(height uint64) (gethCommon.Hash, error) | ||
|
|
||
| // BlockProposal returns the active block proposal | ||
| BlockProposal() (*types.BlockProposal, error) | ||
|
|
||
| // StageBlockProposal updates the in-memory block proposal without writing to | ||
| // storage. Persistence happens at transaction end via FlushBlockProposal. | ||
| StageBlockProposal(*types.BlockProposal) | ||
|
|
||
| // CommitBlockProposal commits the block proposal and update the chain of blocks | ||
| CommitBlockProposal(*types.BlockProposal) error | ||
| } | ||
|
|
||
| const ( | ||
| BlockHashListCapacity = 256 | ||
| BlockStoreLatestBlockKey = "LatestBlock" | ||
| BlockStoreLatestBlockProposalKey = "LatestBlockProposal" | ||
| ) | ||
|
|
||
| // BlockStore manages EVM block storage and block proposal lifecycle during Flow block execution. | ||
| // | ||
| // Storage Keys: | ||
| // - LatestBlock: The last finalized EVM block. Updated only at CommitBlockProposal(). | ||
| // - LatestBlockProposal: The in-progress EVM block accumulating transactions. | ||
| // Its parent hash must equal hash(LatestBlock) and height must equal LatestBlock.Height + 1. | ||
| // | ||
| // Each Cadence transaction creates a new BlockStore instance with an empty cache. | ||
| // The cache avoids repeated storage reads within a single Cadence transaction. | ||
| // | ||
| // Flow Block K Execution: | ||
| // | ||
| // ├── Cadence tx 1 (succeed) | ||
| // │ ├── EVM Tx A | ||
| // │ │ ├── BlockProposal() | ||
| // │ │ │ ├── cache miss | ||
| // │ │ │ ├── read LatestBlockProposal from storage | ||
| // │ │ │ │ └── (if empty) read LatestBlock from storage (for parent hash, height) | ||
| // │ │ │ └── cache it | ||
| // │ │ └── StageBlockProposal() → update cache | ||
| // │ ├── EVM Tx B | ||
| // │ │ ├── BlockProposal() → cache hit | ||
| // │ │ └── StageBlockProposal() → update cache | ||
| // │ └── [tx end] | ||
| // │ └── FlushBlockProposal() → write LatestBlockProposal, cache = nil | ||
| // │ | ||
| // ├── Cadence tx 2 (failed) | ||
| // │ ├── EVM Tx C | ||
| // │ │ ├── BlockProposal() | ||
| // │ │ │ ├── cache miss | ||
| // │ │ │ └── read LatestBlockProposal from storage → cache it | ||
| // │ │ └── StageBlockProposal() → update cache | ||
| // │ ├── EVM Tx D | ||
| // │ │ ├── BlockProposal() → cache hit | ||
| // │ │ └── StageBlockProposal() → update cache | ||
| // │ └── [tx fail/revert] | ||
| // │ └── Reset() → cache = nil, storage unchanged | ||
| // │ | ||
| // └── System chunk tx (last) | ||
| // └── heartbeat() | ||
| // └── CommitBlockProposal() | ||
| // ├── write LatestBlock | ||
| // ├── write new LatestBlockProposal (for next flow block) | ||
| // └── cache = nil | ||
| type BlockStore struct { | ||
| chainID flow.ChainID | ||
| storage ValueStore | ||
| blockInfo BlockInfo | ||
| randGen RandomGenerator | ||
| rootAddress flow.Address | ||
| cached *types.BlockProposal | ||
| } | ||
|
|
||
| var _ EVMBlockStore = &BlockStore{} | ||
|
|
||
| // NewBlockStore constructs a new block store | ||
| func NewBlockStore( | ||
| chainID flow.ChainID, | ||
| storage ValueStore, | ||
| blockInfo BlockInfo, | ||
| randGen RandomGenerator, | ||
| rootAddress flow.Address, | ||
| ) *BlockStore { | ||
| return &BlockStore{ | ||
| chainID: chainID, | ||
| storage: storage, | ||
| blockInfo: blockInfo, | ||
| randGen: randGen, | ||
| rootAddress: rootAddress, | ||
| } | ||
| } | ||
|
|
||
| // BlockProposal returns the block proposal to be updated by the handler | ||
| func (bs *BlockStore) BlockProposal() (*types.BlockProposal, error) { | ||
| if bs.cached != nil { | ||
| return bs.cached, nil | ||
| } | ||
| // first fetch it from the storage | ||
| data, err := bs.storage.GetValue(bs.rootAddress[:], []byte(BlockStoreLatestBlockProposalKey)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if len(data) != 0 { | ||
| bp, err := types.NewBlockProposalFromBytes(data) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| bs.cached = bp | ||
| return bp, nil | ||
| } | ||
| bp, err := bs.constructBlockProposal() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| bs.cached = bp | ||
| return bp, nil | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| func (bs *BlockStore) constructBlockProposal() (*types.BlockProposal, error) { | ||
| // if available construct a new one | ||
| cadenceHeight, err := bs.blockInfo.GetCurrentBlockHeight() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| cadenceBlock, found, err := bs.blockInfo.GetBlockAtHeight(cadenceHeight) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if !found { | ||
| return nil, fmt.Errorf("cadence block not found") | ||
| } | ||
|
|
||
| lastExecutedBlock, err := bs.LatestBlock() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| parentHash, err := lastExecutedBlock.Hash() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // cadence block timestamp is unix nanoseconds but evm blocks | ||
| // expect timestamps in unix seconds so we convert here | ||
| timestamp := uint64(cadenceBlock.Timestamp / int64(time.Second)) | ||
|
|
||
| // read a random value for block proposal | ||
| prevrandao := gethCommon.Hash{} | ||
| err = bs.randGen.ReadRandom(prevrandao[:]) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| blockProposal := types.NewBlockProposal( | ||
| parentHash, | ||
| lastExecutedBlock.Height+1, | ||
| timestamp, | ||
| lastExecutedBlock.TotalSupply, | ||
| prevrandao, | ||
| ) | ||
|
|
||
| return blockProposal, nil | ||
| } | ||
|
|
||
| // UpdateBlockProposal updates the block proposal | ||
| func (bs *BlockStore) updateBlockProposal(bp *types.BlockProposal) error { | ||
| blockProposalBytes, err := bp.ToBytes() | ||
| if err != nil { | ||
| return types.NewFatalError(err) | ||
| } | ||
|
|
||
| return bs.storage.SetValue( | ||
| bs.rootAddress[:], | ||
| []byte(BlockStoreLatestBlockProposalKey), | ||
| blockProposalBytes, | ||
| ) | ||
| } | ||
|
|
||
| // CommitBlockProposal commits the block proposal to the chain | ||
| func (bs *BlockStore) CommitBlockProposal(bp *types.BlockProposal) error { | ||
| bp.PopulateRoots() | ||
|
|
||
| blockBytes, err := bp.Block.ToBytes() | ||
| if err != nil { | ||
| return types.NewFatalError(err) | ||
| } | ||
|
|
||
| err = bs.storage.SetValue(bs.rootAddress[:], []byte(BlockStoreLatestBlockKey), blockBytes) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| hash, err := bp.Block.Hash() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| bhl, err := bs.getBlockHashList() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = bhl.Push(bp.Block.Height, hash) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Construct and store the new block proposal eagerly to maintain | ||
| // state compatibility with the previous implementation. | ||
| newBP, err := bs.constructBlockProposal() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to revert my changes, and keep as is. The optimization of the lazy construction of next block proposal would be incompatible with previous implementation which makes it harder to verify the correctness of this PR. |
||
| if err != nil { | ||
| return err | ||
| } | ||
| err = bs.updateBlockProposal(newBP) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| bs.cached = nil | ||
| return nil | ||
| } | ||
|
|
||
| // LatestBlock returns the latest executed block | ||
| func (bs *BlockStore) LatestBlock() (*types.Block, error) { | ||
| data, err := bs.storage.GetValue(bs.rootAddress[:], []byte(BlockStoreLatestBlockKey)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if len(data) == 0 { | ||
| return types.GenesisBlock(bs.chainID), nil | ||
| } | ||
| return types.NewBlockFromBytes(data) | ||
| } | ||
|
|
||
| // BlockHash returns the block hash for the last x blocks | ||
| func (bs *BlockStore) BlockHash(height uint64) (gethCommon.Hash, error) { | ||
| bhl, err := bs.getBlockHashList() | ||
| if err != nil { | ||
| return gethCommon.Hash{}, err | ||
| } | ||
| _, hash, err := bhl.BlockHashByHeight(height) | ||
| return hash, err | ||
| } | ||
|
|
||
| func (bs *BlockStore) getBlockHashList() (*BlockHashList, error) { | ||
| bhl, err := NewBlockHashList(bs.storage, bs.rootAddress, BlockHashListCapacity) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if bhl.IsEmpty() { | ||
| err = bhl.Push( | ||
| types.GenesisBlock(bs.chainID).Height, | ||
| types.GenesisBlockHash(bs.chainID), | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
||
| return bhl, nil | ||
| } | ||
|
|
||
| func (bs *BlockStore) ResetBlockProposal() { | ||
| bs.cached = nil | ||
| } | ||
|
|
||
| func (bs *BlockStore) StageBlockProposal(bp *types.BlockProposal) { | ||
| bs.cached = bp | ||
| } | ||
|
|
||
| func (bs *BlockStore) FlushBlockProposal() error { | ||
| if bs.cached == nil { | ||
| return nil | ||
| } | ||
| err := bs.updateBlockProposal(bs.cached) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return nil | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlockHashListstill is not cached per transaction.BlockStoreonly keepscached *types.BlockProposal;getBlockHashList()reconstructs the hash list from storage on everyBlockHash()/CommitBlockProposal()path. RepeatedBLOCKHASHaccess inside one Cadence transaction will therefore stay on the metered storage path, which leaves part of the targeted optimization on the table.Possible direction
type BlockStore struct { chainID flow.ChainID storage ValueStore blockInfo BlockInfo randGen RandomGenerator rootAddress flow.Address cached *types.BlockProposal + cachedBlockHashList *BlockHashList } @@ func (bs *BlockStore) getBlockHashList() (*BlockHashList, error) { + if bs.cachedBlockHashList != nil { + return bs.cachedBlockHashList, nil + } + bhl, err := NewBlockHashList(bs.storage, bs.rootAddress, BlockHashListCapacity) if err != nil { return nil, err } @@ if bhl.IsEmpty() { err = bhl.Push( types.GenesisBlock(bs.chainID).Height, types.GenesisBlockHash(bs.chainID), ) if err != nil { return nil, err } } + bs.cachedBlockHashList = bhl return bhl, nil }Also applies to: 224-240
🤖 Prompt for AI Agents