Code storage by hash #5889

jframe · 2023-09-18T07:02:51Z

PR description

Storing code by hash instead of by address. This is needed so that Besu can serve snapsync as we need to be able to retrieve the code by hash.

After syncing with this change it's not possible to downgrade to an older version of Besu. If we think this is needed I could add a subcommand to do the conversion.

Testing

Checkpoint sync on Sepolia, Goerli and Mainnet
Confirmed the same code bytes were stored for both column families by running a comparison tool on the stored code bytes https://github.com/hyperledger/besu/pull/6019/files#diff-9c4d660b9f0b067bbaf230c40ff79f2a53fef78e88c0c5e368b43e3891d0f68bR227
Compatibility testing to ensure that existing databases continue to work
Compared storage of code size on mainnet. Before: 9 GiB After: 2GiB. Column family to store code counts i.e. CODE_HASH_COUNT is using 38 MiB.
Compared FCU and new payload times on mainnet with and without code changes showing no significant difference in times

Fixed Issue(s)

fixed #5388

github-actions · 2023-09-18T07:03:07Z

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

.../main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

Signed-off-by: Jason Frame <[email protected]>

…count for code count updates Signed-off-by: Jason Frame <[email protected]>

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

Signed-off-by: Jason Frame <[email protected]>

…ncremented unnecessarily Signed-off-by: Jason Frame <[email protected]>

Signed-off-by: Jason Frame <[email protected]>

...rc/main/java/org/hyperledger/besu/ethereum/trie/bonsai/storage/flat/CodeStorageStrategy.java

Signed-off-by: Jason Frame <[email protected]>

garyschulte · 2024-01-23T20:30:19Z

...java/org/hyperledger/besu/ethereum/trie/bonsai/storage/flat/CodeHashCodeStorageStrategy.java

+      final SegmentedKeyValueStorageTransaction transaction,
+      final Hash accountHash,
+      final Hash codeHash) {
+    // TODO JF add reference counting so that code can be removed when there are no more usages


👍 , we should like the issue for context

good idea, I'll update the comment with the issue

garyschulte

Looks good, the only significant concern is the ability for code storage CLI param to affect besu's worldstate consistency when it is initially synced with a different code storage method

garyschulte · 2024-01-23T20:38:49Z

...ore/src/main/java/org/hyperledger/besu/ethereum/trie/bonsai/storage/flat/FlatDbStrategy.java

+    this.codeStorageStrategy =
+        useCodeStoredByHash
+            ? new CodeHashCodeStorageStrategy()
+            : new AccountHashCodeStorageStrategy();



can we move this up into FlatDbStrategyProvider, and pass a CodeStorageStrategy rather than a boolean ?

garyschulte · 2024-01-23T20:50:06Z

...main/java/org/hyperledger/besu/ethereum/trie/bonsai/storage/flat/FlatDbStrategyProvider.java

+      final boolean isCodeStoredByHashEnabled =
+          dataStorageConfiguration.getUnstable().getBonsaiCodeStoredByCodeHashEnabled();
+      LOG.info(
+          "Bonsai flat db mode with code stored using code hash enabled = {}",
+          isCodeStoredByHashEnabled);


the risk of driving this from command line only is that we can fall out of consensus if the user changes this config parameter without resyncing. It isn't fun to plumb in the check but we should try to derive the codehash vs accounthash as it exists in the database and only allow the cli parameter to affect an as-yet-unsynced node.

we already have the notion of using loadFlatDbStrategy(..) in BonsaiWorldStateKeyValueStorage, and it will override the CLI supplied flat db strategy based on what it finds. We can work a code storage check into that method and ensure that we use the correct code storage strategy based on what is in the db, and perhaps log a big ugly warning message if the CLI param does not match the db storage

Added detection code in the FlatDbStrategyProvider which will warn if there is an inconsistency and use the strategy in the database.

garyschulte · 2024-01-23T20:56:13Z

.../src/test/java/org/hyperledger/besu/ethereum/trie/bonsai/worldview/BonsaiWorldStateTest.java

+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class BonsaiWorldStateTest {


IMO we should add a test that asserts the correct FlatDBStrategy (with the correct CodeStorageStrategy) is constructed when the db storage and the cli param disagree.

Added some tests for that in the FlatDbStrategyProvider since that's where it's created

Signed-off-by: Jason Frame <[email protected]>

garyschulte

🚢 great test coverage

non-fungible-nelson · 2024-01-25T20:24:09Z

💯 🚀

This reverts commit 9c02518. Signed-off-by: Jason Frame <[email protected]>

Add a storage mode for Bonsai that can store the code by hash instead of account hash Signed-off-by: Jason Frame <[email protected]>

Add a storage mode for Bonsai that can store the code by hash instead of account hash Signed-off-by: Jason Frame <[email protected]> Signed-off-by: Gabriel-Trintinalia <[email protected]>

…6504) This reverts commit 9c02518. Signed-off-by: Jason Frame <[email protected]> Signed-off-by: Gabriel-Trintinalia <[email protected]>

github-advanced-security bot found potential problems Sep 21, 2023

View reviewed changes

.../main/java/org/hyperledger/besu/ethereum/bonsai/storage/BonsaiWorldStateKeyValueStorage.java Fixed Show fixed Hide fixed

jframe force-pushed the code_storage_by_hash branch 2 times, most recently from b254b42 to 325fc54 Compare September 22, 2023 05:57

garyschulte mentioned this pull request Sep 25, 2023

SnapServer using FlatDB Strategy #5887

Closed

jframe force-pushed the code_storage_by_hash branch 3 times, most recently from a4d26f8 to ea8893e Compare September 29, 2023 00:00

garyschulte added a commit to garyschulte/besu that referenced this pull request Oct 5, 2023

SHOULD NOT MERGE TO MAIN. blunt change to save code by hash until hyp…

ebc5dc7

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

garyschulte added a commit to garyschulte/besu that referenced this pull request Oct 5, 2023

SHOULD NOT MERGE TO MAIN. blunt change to save code by hash until hyp…

0e0c184

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

garyschulte added a commit to garyschulte/besu that referenced this pull request Oct 5, 2023

SHOULD NOT MERGE TO MAIN. blunt change to save code by hash until hyp…

ef5f1ca

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

garyschulte added a commit to garyschulte/besu that referenced this pull request Oct 5, 2023

SHOULD NOT MERGE TO MAIN. blunt change to save code by hash until hyp…

2443334

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

Code storage by keyed by code hash

f68da4e

Signed-off-by: Jason Frame <[email protected]>

jframe force-pushed the code_storage_by_hash branch from d254d70 to f68da4e Compare October 6, 2023 03:57

jframe added 5 commits October 6, 2023 14:02

remove unused function

548a2f8

Signed-off-by: Jason Frame <[email protected]>

code storage legacy tests

0aa8ff9

Signed-off-by: Jason Frame <[email protected]>

code storage unit tests and Byte32 -> Hash refactor

1e803e5

Signed-off-by: Jason Frame <[email protected]>

add get method to SegmentedKeyValueStorageTransaction

30ee2d7

Signed-off-by: Jason Frame <[email protected]>

use get method on transaction so that pending updates update correct …

ec862a6

…count for code count updates Signed-off-by: Jason Frame <[email protected]>

garyschulte added a commit to garyschulte/besu that referenced this pull request Oct 10, 2023

SHOULD NOT MERGE TO MAIN. blunt change to save code by hash until hyp…

bb69e2a

…erledger#5889. Signed-off-by: garyschulte <[email protected]>

jframe added 3 commits October 11, 2023 10:34

unit tests for BonsaiWorldState updateCode

2413afd

Signed-off-by: Jason Frame <[email protected]>

fix compile error on PersistDataStepTest

2f051ce

Signed-off-by: Jason Frame <[email protected]>

fix early return in BonsaiWorldState

b1465d5

Signed-off-by: Jason Frame <[email protected]>

jframe mentioned this pull request Oct 11, 2023

Codehash compare #6019

Closed

jframe added 3 commits October 13, 2023 12:20

prevent any updates when code hasn't changed to ensure count is not i…

9117771

…ncremented unnecessarily Signed-off-by: Jason Frame <[email protected]>

additional tests for putCode in BonsaiWorldState

27bab00

Signed-off-by: Jason Frame <[email protected]>

only store code value if it doesn't already exist

387d8e4

Signed-off-by: Jason Frame <[email protected]>

jframe changed the title ~~Draft PR: Code storage by hash~~ Code storage by hash Oct 16, 2023

jframe marked this pull request as ready for review October 16, 2023 04:14

jframe requested a review from garyschulte October 16, 2023 04:14

garyschulte mentioned this pull request Jan 12, 2024

worldstate refactor #6209

Merged

jframe added 3 commits January 16, 2024 10:55

remove code hash strategy deletion

ff89cf6

Signed-off-by: Jason Frame <[email protected]>

remove rocksdb transaction get method

44739e2

Signed-off-by: Jason Frame <[email protected]>

revert plugin-api checksum change

05862bb

Signed-off-by: Jason Frame <[email protected]>

github-advanced-security bot found potential problems Jan 16, 2024

View reviewed changes

...rc/main/java/org/hyperledger/besu/ethereum/trie/bonsai/storage/flat/CodeStorageStrategy.java Dismissed Show dismissed Hide dismissed

jframe added 5 commits January 19, 2024 14:32

Merge remote-tracking branch 'upstream/main' into code_storage_by_hash

2a184da

reapply code strategy changes after merge

673ac35

Signed-off-by: Jason Frame <[email protected]>

cleanup

d11b82f

Signed-off-by: Jason Frame <[email protected]>

add log for using storage by code hash

bc2018e

Signed-off-by: Jason Frame <[email protected]>

Merge remote-tracking branch 'upstream/main' into code_storage_by_hash

5406e97

garyschulte reviewed Jan 23, 2024

View reviewed changes

garyschulte requested changes Jan 23, 2024

View reviewed changes

jframe added 4 commits January 24, 2024 08:01

remove extra field in DataStorageOptions due to bad merge

4ce81b0

Signed-off-by: Jason Frame <[email protected]>

Use code storage mode from database

ae5a968

Signed-off-by: Jason Frame <[email protected]>

Update comment with issue

ba3f92e

Signed-off-by: Jason Frame <[email protected]>

spotless

825a1d9

Signed-off-by: Jason Frame <[email protected]>

garyschulte approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into code_storage_by_hash

b390f7a

jframe enabled auto-merge (squash) January 30, 2024 22:27

jframe disabled auto-merge January 30, 2024 22:43

jframe enabled auto-merge (squash) January 30, 2024 22:44

Merge branch 'main' into code_storage_by_hash

8b74e85

jframe merged commit 9c02518 into hyperledger:main Jan 31, 2024
18 checks passed

jframe added a commit to jframe/besu that referenced this pull request Jan 31, 2024

Revert "Bonsai code storage by hash (hyperledger#5889)"

74c69bb

This reverts commit 9c02518. Signed-off-by: Jason Frame <[email protected]>

jframe added a commit that referenced this pull request Jan 31, 2024

Revert "Bonsai code storage by hash (#5889)" (#6504)

4252e47

This reverts commit 9c02518. Signed-off-by: Jason Frame <[email protected]>

jframe added a commit to jframe/besu that referenced this pull request Jan 31, 2024

Bonsai code storage by hash (hyperledger#5889)

0f52f0a

Add a storage mode for Bonsai that can store the code by hash instead of account hash Signed-off-by: Jason Frame <[email protected]>

jframe mentioned this pull request Jan 31, 2024

Code storage by hash v2 #6505

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code storage by hash #5889

Code storage by hash #5889

jframe commented Sep 18, 2023 •

edited

Loading

github-actions bot commented Sep 18, 2023 •

edited by jframe

Loading

garyschulte Jan 23, 2024 •

edited

Loading

jframe Jan 23, 2024

garyschulte left a comment

garyschulte Jan 23, 2024

jframe Jan 25, 2024

garyschulte Jan 23, 2024 •

edited

Loading

jframe Jan 25, 2024

garyschulte Jan 23, 2024

jframe Jan 25, 2024

garyschulte left a comment

non-fungible-nelson commented Jan 25, 2024

Code storage by hash #5889

Code storage by hash #5889

Conversation

jframe commented Sep 18, 2023 • edited Loading

PR description

Fixed Issue(s)

github-actions bot commented Sep 18, 2023 • edited by jframe Loading

garyschulte Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

jframe Jan 23, 2024

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

garyschulte Jan 23, 2024

Choose a reason for hiding this comment

jframe Jan 25, 2024

Choose a reason for hiding this comment

garyschulte Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

jframe Jan 25, 2024

Choose a reason for hiding this comment

garyschulte Jan 23, 2024

Choose a reason for hiding this comment

jframe Jan 25, 2024

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

non-fungible-nelson commented Jan 25, 2024

jframe commented Sep 18, 2023 •

edited

Loading

github-actions bot commented Sep 18, 2023 •

edited by jframe

Loading

garyschulte Jan 23, 2024 •

edited

Loading

garyschulte Jan 23, 2024 •

edited

Loading