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

fix collision issue #6205

Merged
merged 7 commits into from
Dec 12, 2023
Merged

fix collision issue #6205

merged 7 commits into from
Dec 12, 2023

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Nov 23, 2023

PR description

The idea of this PR is to revert some modification that was made to pass tests #5686.

Therefore, I decided to revert some modification on this PR by fixing the problem differently. Indeed, the idea of the collision test that does not pass is to selfdestruct a contract and recreate it afterwards. Instead of modifying the logic of the accumulator and listing all the storage leaves to be deleted during the selfdestruct, which seems to lead to edge cases, I prefer to let BonsaiWorldstate handle it as before and just load the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation. Indeed, the clearing of the storage that is done at the beginning of calculateRootHash removes the storage but does not change the storage root of the contract. Therefore, we falsely believe that the storage is not empty.

Fixed Issue(s)

Signed-off-by: Karim TAAM <[email protected]>
Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

This will be faster and simpler, but at the cost of leaving dangling storage slots on self destruct right?

This seems like a viable tradeoff especially in light of EIP-6780. I just want to clarify

@garyschulte garyschulte marked this pull request as ready for review December 1, 2023 17:40
@matkt
Copy link
Contributor Author

matkt commented Dec 11, 2023

Sync passed on goerli @garyschulte if you can review it please

Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

Seems good 👍

@matkt matkt merged commit e90b4f1 into hyperledger:main Dec 12, 2023
18 checks passed
matkt added a commit that referenced this pull request Dec 12, 2023
…#6205)

revert some modification that was made to pass tests #5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
matthew1001 added a commit that referenced this pull request Dec 12, 2023
* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (#6205)

revert some modification that was made to pass tests #5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation. 

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 12, 2023
…hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 12, 2023
* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>
jflo added a commit that referenced this pull request Dec 12, 2023
* New cli options to limit rewards return by eth_feeHistory  (#6202)
* [#5851] Add error messages on authentication failures with username and password (#6212)
* Add a constant for the 'password'
* Add error messages on authentication failures with username and password

Signed-off-by: Justin Florentine <[email protected]>
Signed-off-by: jflo <[email protected]>
Signed-off-by: Gabriel-Trintinalia <[email protected]>
Signed-off-by: David Lutzardo <[email protected]>



Signed-off-by: David Lutzardo <[email protected]>

* Add test to check empty login and check response in body is not empty

Signed-off-by: David Lutzardo <[email protected]>

* Correct format (spotless)

Signed-off-by: David Lutzardo <[email protected]>

* Update ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceLoginTest.java

Co-authored-by: Fabio Di Fabio <[email protected]>
Signed-off-by: David Lutzardo <[email protected]>

* Update ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceLoginTest.java

Co-authored-by: Fabio Di Fabio <[email protected]>
Signed-off-by: David Lutzardo <[email protected]>

* Update JsonRpcHttpServiceLoginTest.java

use containsIgnoringCase

Signed-off-by: David Lutzardo <[email protected]>

* Add a CHANGELOG entry for PR 6212

Signed-off-by: David Lutzardo <[email protected]>

---------

Signed-off-by: David Lutzardo <[email protected]>
Co-authored-by: Fabio Di Fabio <[email protected]>
Signed-off-by: jflo <[email protected]>

* Add RockDB Subcommand for printing usage per column family (#6185)

* Add RockDB Subcommand for printing usage per column family
Signed-off-by: Simon Dudley <[email protected]>

* changed output to follow a MD table notation.

Signed-off-by: Gabriel Fukushima <[email protected]>

---------

Signed-off-by: Gabriel Fukushima <[email protected]>
Co-authored-by: Gabriel Fukushima <[email protected]>
Signed-off-by: jflo <[email protected]>

* Deprecation warning if Forest pruning is enabled (#6230)

Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: jflo <[email protected]>

* Fix the annoying "Errors occurred while build effective model" during builds (#6241)

Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: jflo <[email protected]>

* Run ATs sequentially (#6244)

Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: jflo <[email protected]>

* fix: double calls to trace{Start,End}Transaction (#6247)

Signed-off-by: delehef <[email protected]>
Signed-off-by: jflo <[email protected]>

* migrate to junit5 (#6234)

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* fixes for problems discovered in main (#6248)

Signed-off-by: garyschulte <[email protected]>
Signed-off-by: jflo <[email protected]>

* Pki - migrate to junit 5 (#6235)

* migrate to junit5

Signed-off-by: Sally MacFarlane <[email protected]>

* fix: double calls to trace{Start,End}Transaction (#6247)

Signed-off-by: Franklin Delehelle <[email protected]>

* migrate to junit5 (#6234)

Signed-off-by: Sally MacFarlane <[email protected]>

* fixes for problems discovered in main (#6248)

Signed-off-by: garyschulte <[email protected]>

* fixed test comparing size of collection

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: Franklin Delehelle <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Co-authored-by: delehef <[email protected]>
Co-authored-by: garyschulte <[email protected]>
Signed-off-by: jflo <[email protected]>

* junit 5 ftw (#6253)

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* removed unnecessary use of static temp dir  (#6251)

* don't use static tempdir

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* Remove parallelism usage from mainnet AT (#6252)

* Remove parallelism usage from mainnet AT

Signed-off-by: Gabriel Fukushima <[email protected]>

* Increase parallelism usage from mainnet AT

Signed-off-by: Gabriel Fukushima <[email protected]>

* Add the split command back

Signed-off-by: Gabriel Fukushima <[email protected]>

---------

Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: jflo <[email protected]>

* fix log params (#6254)

Signed-off-by: Sally MacFarlane <[email protected]>
Co-authored-by: Stefan Pingel <[email protected]>
Signed-off-by: jflo <[email protected]>

* add dependency on jar task (#6255)

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* Fix and test that the BlockAwareOperationTracer methods are invoked the correct number of times (#6259)

* Test that the BlockAwareOperationTracer are invoked the correct number of times
* Remove redundant calls to traceEndBlock

Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: jflo <[email protected]>

* [RPC] Use apiConfiguration to limit gasPrice in eth_getGasPrice (#6243)

Signed-off-by: Gabriel-Trintinalia <[email protected]>
Signed-off-by: jflo <[email protected]>

* log bootnodes and static nodes list at debug level (#6273)

* log bootnodes and static nodes list at debug level

Signed-off-by: Sally MacFarlane <[email protected]>

* log if zero bootnodes

Signed-off-by: Sally MacFarlane <[email protected]>

* null safeguards

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* junit 5 (#6256)

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* Non bft group ats junit 5 (#6260)

* migrate to junit 5

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* Txparse subcommand implementation (#6268)

* txparse subcommand

Signed-off-by: garyschulte <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* [MINOR] migrate remaining Crypto tests to junit 5 (#6280)

* update crypto tests to junit5

* fixed temp file

* removed vintage junit dep

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: jflo <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (#6205)

revert some modification that was made to pass tests #5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>

* Sequenced pool synonym for legacy pool (#6274)

* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (#6205)

revert some modification that was made to pass tests #5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>

* ETC mainnet 'Spiral' activation block (#6267)

* Set ENR tree for DNS discovery for ETC mainnet network

Signed-off-by: Diego López León <[email protected]>

* Set activation block number for ECIP-1109 on ETC mainnet

Signed-off-by: Diego López León <[email protected]>

---------

Signed-off-by: Diego López León <[email protected]>
Signed-off-by: jflo <[email protected]>

* uprev to version 23.10.3-RC3

Signed-off-by: jflo <[email protected]>

---------

Signed-off-by: Justin Florentine <[email protected]>
Signed-off-by: jflo <[email protected]>
Signed-off-by: Gabriel-Trintinalia <[email protected]>
Signed-off-by: David Lutzardo <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: delehef <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: Franklin Delehelle <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Diego López León <[email protected]>
Co-authored-by: Gabriel-Trintinalia <[email protected]>
Co-authored-by: David Lutzardo <[email protected]>
Co-authored-by: Fabio Di Fabio <[email protected]>
Co-authored-by: Simon Dudley <[email protected]>
Co-authored-by: Gabriel Fukushima <[email protected]>
Co-authored-by: delehef <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: garyschulte <[email protected]>
Co-authored-by: Stefan Pingel <[email protected]>
Co-authored-by: matkt <[email protected]>
Co-authored-by: Matt Whitehead <[email protected]>
Co-authored-by: Diego López León <[email protected]>
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
…hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation. 

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
…hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
gfukushima pushed a commit to gfukushima/besu that referenced this pull request Dec 15, 2023
* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
Signed-off-by: Gabriel Fukushima <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 18, 2023
…hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>
jflo pushed a commit to jflo/besu that referenced this pull request Dec 18, 2023
* Sequenced pool synonym for legacy pool

Signed-off-by: Matthew Whitehead <[email protected]>

* Class rename

Signed-off-by: Matthew Whitehead <[email protected]>

* Spotless fixes

Signed-off-by: Matthew Whitehead <[email protected]>

* Add SEQUENCED to config overview test

Signed-off-by: Matthew Whitehead <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>

* add a fix to load correctly the storage trie in the Bonsai WorldState (hyperledger#6205)

revert some modification that was made to pass tests hyperledger#5686 and fix this tests by loading the storage with EMPTY_TRIE_HASH if we detect that it has been cleared before pushing the new slots after recreation.

---------

Signed-off-by: Karim TAAM <[email protected]>

---------

Signed-off-by: Matthew Whitehead <[email protected]>
Signed-off-by: Matt Whitehead <[email protected]>
Signed-off-by: Karim TAAM <[email protected]>
Co-authored-by: Sally MacFarlane <[email protected]>
Co-authored-by: matkt <[email protected]>
Signed-off-by: jflo <[email protected]>
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.

2 participants