Skip to content
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

Add offchain block context creation method #6751

Merged
merged 9 commits into from
Nov 22, 2024

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Nov 21, 2024

This adds a way to create block context used for re-execution so that the block context contains historically correct data.

This addresses the Block Hash list issue and the coinbase issue. more details in the comments of the code.

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 30.00000% with 42 lines in your changes missing coverage. Please review.

Project coverage is 43.23%. Comparing base (a3676ba) to head (cadbde9).

Files with missing lines Patch % Lines
fvm/evm/offchain/blocks/block_context.go 32.72% 37 Missing ⚠️
fvm/evm/offchain/blocks/blocks.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6751      +/-   ##
==========================================
+ Coverage   41.23%   43.23%   +1.99%     
==========================================
  Files        2060     1586     -474     
  Lines      182635   146021   -36614     
==========================================
- Hits        75316    63128   -12188     
+ Misses     101011    77665   -23346     
+ Partials     6308     5228    -1080     
Flag Coverage Δ
unittests 43.23% <30.00%> (+1.99%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@janezpodhostnik janezpodhostnik marked this pull request as ready for review November 21, 2024 13:32
fvm/evm/offchain/blocks/block_context.go Outdated Show resolved Hide resolved
fvm/evm/offchain/blocks/block_context.go Outdated Show resolved Hide resolved

// For testnet & mainnet, we fetch the block hash from the hard-coded
// array of hashes.
if chainID == flow.Mainnet && height < blockHashListFixHCUEVMHeightMainnet {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add this logic to BasicProvider.OnBlockExecuted to call PushBlockHash with the hard-coded array of hashes.

So that my backward compatibility test will be able to save with the hard-coded hashes, and verify the trie update matches. If it matches, then we know these hard-coded configs are correct and can be used by gateway.

Copy link
Member

@zhangchiqing zhangchiqing Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a method similar to what you had in your branch

Copy link
Member

@zhangchiqing zhangchiqing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@janezpodhostnik janezpodhostnik added this pull request to the merge queue Nov 22, 2024
Merged via the queue into master with commit b3d0864 Nov 22, 2024
55 checks passed
@janezpodhostnik janezpodhostnik deleted the janez/offchain-block-context branch November 22, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants