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.

Sign up for GitHub

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

Jump to bottom

core/state: do not ignore null addr while iterative dump #27320

Merged
merged 4 commits into from
May 23, 2023
Merged

core/state: do not ignore null addr while iterative dump #27320

merged 4 commits into from
May 23, 2023

Conversation

pcw109550
Copy link
Contributor

This PR fixes bug which null address is ignored while doing iterative dump.
Null address is excluded in the jsonl result, which makes preimage(null address) not included in the final output.

This PR also adds test for testing iterative dump functionality.

Example

Iterative dump without the fix:

{"root":"0xd5710ea8166b7b04bc2bfb129d7db12931cee82f75ca8e2d075b4884322bf3de"}
{"balance":"22","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","address":"0x0000000000000000000000000000000000000001","key":"0x1468288056310c82aa4c01a7e12a10f8111a0560e72b700555479031b86c357d"}
{"balance":"1337","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","key":"0x5380c7b7ae81a58eb98d9c78de4a1fd7fd9535fc953ed2be602daaa41767312a"}
{"balance":"0","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0x87874902497a5bb968da31a2998d8f22e949d1ef6214bcdedd8bae24cca4b9e3","code":"0x03030303030303","address":"0x0000000000000000000000000000000000000102","key":"0xa17eacbc25cda025e81db9c5c62868822c73ce097cee2a63e33a2e41268358a1"}     
{"balance":"44","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","address":"0x0000000000000000000000000000000000000002","key":"0xd52688a8f926c816ca1e079067caba944f158e764817b83fc43594370ca9cf62"}

You may observe that address value is not included for null address(third line of jsonl output).

Iterative dump with the fix:

{"root":"0xd5710ea8166b7b04bc2bfb129d7db12931cee82f75ca8e2d075b4884322bf3de"}
{"balance":"22","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","address":"0x0000000000000000000000000000000000000001","key":"0x1468288056310c82aa4c01a7e12a10f8111a0560e72b700555479031b86c357d"}
{"balance":"1337","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","address":"0x0000000000000000000000000000000000000000","key":"0x5380c7b7ae81a58eb98d9c78de4a1fd7fd9535fc953ed2be602daaa41767312a"}
{"balance":"0","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0x87874902497a5bb968da31a2998d8f22e949d1ef6214bcdedd8bae24cca4b9e3","code":"0x03030303030303","address":"0x0000000000000000000000000000000000000102","key":"0xa17eacbc25cda025e81db9c5c62868822c73ce097cee2a63e33a2e41268358a1"}
{"balance":"44","nonce":0,"root":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421","codeHash":"0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470","address":"0x0000000000000000000000000000000000000002","key":"0xd52688a8f926c816ca1e079067caba944f158e764817b83fc43594370ca9cf62"}

holiman
holiman reviewed May 22, 2023
View reviewed changes
core/state/dump.go Outdated
if addr != (common.Address{}) {
dumpAccount.Address = &addr
}
dumpAccount.Address = &addr
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't good either, because the addr is not a pointer, so the "we don't have it" null-element is the zero-address.
So as it is on master, it will fail by not showing the 0x000..0 -address in the output. Which is wrong.
In this PR, however, it will fail by showing accounts for which we do not have the preimage (address), as if they were 0x00..00.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed, ptal

Copy link
Contributor Author

@pcw109550 pcw109550 May 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I understand the fix, but the fix will break t8n: https://github.com/ethereum/go-ethereum/tree/master/cmd/evm:

go-ethereum/cmd/evm/internal/t8ntool/transition.go

Line 303 in 5b792e0

s.DumpToCollector(collector, nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed to make evmtools happy.

@pcw109550 pcw109550 requested a review from holiman May 22, 2023 17:40
holiman
holiman approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman added this to the 1.11.7 milestone May 23, 2023
@holiman holiman merged commit bfded65 into ethereum:master May 23, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@pcw109550 @holiman @devopsbo3
core/state: do not ignore null addr while iterative dump (ethereum#27320
8686762 
)

fixes bug which caused the zero-address to be ignored during an iterative state-dump.

---------

Co-authored-by: Martin Holst Swende <martin@swende.se>
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "core/state: do not ignore null addr while iterative dump (eth…
c226481 
…ereum#27320)"

This reverts commit 8686762.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "core/state: do not ignore null addr while iterative dump (eth…
33d85d1 
…ereum#27320)"

This reverts commit 8686762.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holiman holiman holiman approved these changes

@karalabe karalabe Awaiting requested review from karalabe karalabe is a code owner

@rjl493456442 rjl493456442 Awaiting requested review from rjl493456442 rjl493456442 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.
2 participants
@pcw109550 @holiman