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

Move blockHash calculation after deduction of supply diff #5462

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Feb 27, 2024

The evm.BlockExecuted.hash & evm.TransactionExecuted.blockHash fields did not match up, e.g.

Index    5
Type    evm.TransactionExecuted
Tx ID    67244b4d6f7e8ed4a5687099386e66424ec8d6adfa1abf739b9b227beb544b24
Values
    - blockHeight (UInt64): 2 
    - blockHash (String): "0x5d85046405a803acf1920cfafb7508f1817f7c905a616e012ea0420704f68c09" 
    - transactionHash (String): "0x95f00658e23f24928fe5a1c4af91df8fdd7b4cf02ea63cc86911d197dfa7912e" 
    - transaction (String): "fff83c81ff0194000000000000000000000000000000000000000094000000000000000000000002fcb52dccbbaa997780891b1ae4d6e2ef50000082520880" 
    - failed (Bool): false 
    - vmError (String): "" 
    - transactionType (UInt8): 255 
    - gasConsumed (UInt64): 21000 
    - deployedContractAddress (String): "0000000000000000000000000000000000000000" 
    - returnedValue (String): "" 
    - logs (String): "" 

Index    6
Type    evm.BlockExecuted
Tx ID    67244b4d6f7e8ed4a5687099386e66424ec8d6adfa1abf739b9b227beb544b24
Values
    - height (UInt64): 2 
    - hash (String): "0x4131fe910a710d9d19128f135f78d33bcf1d4c50d5a6b9b1cd0b3a410539f3bd" 
    - totalSupply (Int): 500000000000000000000 
    - parentHash (String): "0x20fce2ec38f2fe8df9de507d5969487152faae0033a23b2cef1f4ab938af7ef2" 
    - receiptRoot (String): "0x0000000000000000000000000000000000000000000000000000000000000000" 
    - transactionHashes ([String]): ["0x95f00658e23f24928fe5a1c4af91df8fdd7b4cf02ea63cc86911d197dfa7912e"] 

This was due to calculating the block hash, before supply diff deduction. The TotalSupply field of a block would change, hence resulting in a different hash value.

@m-Peter m-Peter changed the title Move blockHash calculation after deduction supply diff Move blockHash calculation after deduction of supply diff Feb 27, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 57.95%. Comparing base (bf3cec4) to head (aa0c6b4).

Files Patch % Lines
fvm/evm/handler/handler.go 25.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           feature/stable-cadence    #5462      +/-   ##
==========================================================
+ Coverage                   56.39%   57.95%   +1.55%     
==========================================================
  Files                        1032      861     -171     
  Lines                      100711    85892   -14819     
==========================================================
- Hits                        56796    49778    -7018     
+ Misses                      39606    32283    -7323     
+ Partials                     4309     3831     -478     
Flag Coverage Δ
unittests 57.95% <25.00%> (+1.55%) ⬆️

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.

@janezpodhostnik janezpodhostnik merged commit 97775ab into onflow:feature/stable-cadence Feb 27, 2024
50 of 51 checks passed
@ramtinms
Copy link
Contributor

Hey @m-Peter do you mind also make a port PR to master, we are going to eventually merge the stable cadence branch but doing it in smaller PRs makes the backport easier.

@m-Peter
Copy link
Collaborator Author

m-Peter commented Feb 27, 2024

@ramtinms On it.

@m-Peter m-Peter deleted the fix-block-hash-inconsistency branch March 20, 2024 07:50
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.

4 participants