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

state: make EIP-158 ignore history block hash contract address #359

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

jsign
Copy link
Collaborator

@jsign jsign commented Feb 2, 2024

This PR fixes a bug regarding the post-value of the history block hash contract being unchanged.

The reason is that EIP-158 was (correctly) deleting the account for this address since the code, balance, and nonce are zero.

I've confirmed this reproduced in the current test and added extra checks to enforce that it can't happen again (since the tests assertions where checking that the key was in the witness, but not the post-state value expectation).

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
@jsign jsign changed the title state: EIP-158 ignore history block hash address state: make EIP-158 ignore history block hash address Feb 2, 2024
@jsign jsign changed the title state: make EIP-158 ignore history block hash address state: make EIP-158 ignore history block hash contract address Feb 2, 2024
Copy link
Owner

@gballet gballet left a comment

Choose a reason for hiding this comment

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

I was surprised at first because I made about the same test, and it was saying that everything worked. And this morning I realize that I had swapped the == and != 🤦‍♂️ I will commit my fixed test as well, even though it's kind of redundant, Testing never hurts. LGTM and thank you for giving it a look 🤝

@@ -682,6 +682,12 @@ func TestProcessVerkleiInvalidContractCreation(t *testing.T) {
if stemStateDiff.SuffixDiffs[0].Suffix != 65 {
t.Fatalf("invalid suffix diff value found for BLOCKHASH contract at block #2: %d != 65", stemStateDiff.SuffixDiffs[0].Suffix)
}
if stemStateDiff.SuffixDiffs[0].NewValue == nil {
Copy link
Owner

Choose a reason for hiding this comment

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

That's strange because that's what I did yesterday as well and it was finding non-nil. That was for the first block. But I see the tests passing so it's got to be correct.

@gballet gballet merged commit 4efa34d into kaustinen-with-shapella Feb 3, 2024
2 of 3 checks passed
gballet pushed a commit that referenced this pull request May 7, 2024
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet pushed a commit that referenced this pull request May 8, 2024
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet pushed a commit that referenced this pull request May 8, 2024
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
gballet pushed a commit that referenced this pull request May 8, 2024
Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
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.

None yet

2 participants