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

cmd/evm: validate blockchain tests poststate account storage #28443

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions tests/block_test_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,12 @@ func (t *BlockTest) validatePostState(statedb *state.StateDB) error {
if nonce2 != acct.Nonce {
return fmt.Errorf("account nonce mismatch for addr: %s want: %d have: %d", addr, acct.Nonce, nonce2)
}
for k, v := range acct.Storage {
v2 := statedb.GetState(addr, k)
if v2 != v {
return fmt.Errorf("account storage mismatch for addr: %s, slot: %x, want: %x, have: %x", addr, k, v, v2)
Comment on lines +334 to +336
Copy link
Contributor

@holiman holiman Oct 31, 2023

Choose a reason for hiding this comment

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

Suggested change
v2 := statedb.GetState(addr, k)
if v2 != v {
return fmt.Errorf("account storage mismatch for addr: %s, slot: %x, want: %x, have: %x", addr, k, v, v2)
if have := statedb.GetState(addr, k); have != v {
return fmt.Errorf("account storage mismatch for addr: %s, slot: %x, want: %x, have: %x", addr, k, v, have)

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with v2 to match the style of the surrounding code, but I also prefer have. What do you think about renaming the other variables, e.g., code2 -> have_code, for consistency?

}
}
Copy link
Member

Choose a reason for hiding this comment

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

We should probably also add acct.RootHash?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with @holiman's comment below, this is tested via the stateRoot specified in the final block in the blocks section of the test fixture.

}
return nil
}
Expand Down