Cache EVM block proposal and defer storage writes to commit #8491
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 | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||
| package environment | ||||||
|
|
||||||
| import ( | ||||||
| "fmt" | ||||||
|
|
@@ -10,6 +10,25 @@ | |||||
| "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" | ||||||
|
|
@@ -18,55 +37,66 @@ | |||||
|
|
||||||
| 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 | ||||||
| } | ||||||
|
|
@@ -90,7 +120,7 @@ | |||||
|
|
||||||
| // read a random value for block proposal | ||||||
| prevrandao := gethCommon.Hash{} | ||||||
| err = bs.randGen.ReadRandom(prevrandao[:]) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
|
|
@@ -107,13 +137,13 @@ | |||||
| } | ||||||
|
|
||||||
| // 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, | ||||||
|
|
@@ -129,7 +159,7 @@ | |||||
| return types.NewFatalError(err) | ||||||
| } | ||||||
|
|
||||||
| err = bs.storage.SetValue(bs.rootAddress[:], []byte(BlockStoreLatestBlockKey), blockBytes) | ||||||
| if err != nil { | ||||||
| return err | ||||||
| } | ||||||
|
|
@@ -153,17 +183,17 @@ | |||||
| if err != nil { | ||||||
| return err | ||||||
| } | ||||||
| err = bs.updateBlockProposal(newBP) | ||||||
| if err != nil { | ||||||
| return err | ||||||
| } | ||||||
| bs.cached = newBP | ||||||
|
Member
There was a problem hiding this comment. I think we can clear the cache after the block proposal is committed, because it won't be used any more.
Suggested change
The CommitBlockProposal is used at the system tx, which is the last tx in a flow block, so there is no more tx to read this cache. There are two keys in storage:
The relationship is that IMO, the CommitBlockProposal could just remove the
Member
There was a problem hiding this comment.
Member
There was a problem hiding this comment. I made a PR for this change and added some test cases |
||||||
| 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 | ||||||
| } | ||||||
|
|
@@ -184,7 +214,7 @@ | |||||
| } | ||||||
|
|
||||||
| func (bs *BlockStore) getBlockHashList() (*BlockHashList, error) { | ||||||
| bhl, err := NewBlockHashList(bs.storage, bs.rootAddress, BlockHashListCapacity) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
|
|
@@ -201,3 +231,23 @@ | |||||
|
|
||||||
| 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 | ||||||
| } | ||||||
|
|
||||||
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
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