Code storage by hash v2 #6505

jframe · 2024-01-31T07:27:17Z

PR description

This is a redo of #5889 including a fix for detection of existing storage.

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

Add a storage mode for Bonsai that can store the code by hash instead of account hash Signed-off-by: Jason Frame <jason.frame@consensys.net>

Signed-off-by: Jason Frame <jason.frame@consensys.net>

github-actions · 2024-01-31T07:27:32Z

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

fab-10 · 2024-01-31T13:35:34Z

Hi @jframe, since this PR changes the format, I think we need to introduce a new version in the database metadata.
When I introduced the variables storage, I didn't introduce a new version, an that was an error, so I am now doing that, and with the occasion, refactoring the way we store database metadata to be more easy to read, so I suggest that we synchronize on this and we can introduce a new version and so leverage the existing protection mechanism.

jframe · 2024-01-31T21:49:39Z

Hi @jframe, since this PR changes the format, I think we need to introduce a new version in the database metadata. When I introduced the variables storage, I didn't introduce a new version, an that was an error, so I am now doing that, and with the occasion, refactoring the way we store database metadata to be more easy to read, so I suggest that we synchronize on this and we can introduce a new version and so leverage the existing protection mechanism.

I think it is a good idea. Will sync up with you on this. Don't think it needs to go in this PR though as this is experimental and doesn't seem right to bake in database versions as part of an experimental feature. But before it's production ready would like to have database versioning for code storage column family in Bonsai to prevent db compatibility issues.

jframe · 2024-02-02T00:05:21Z

Confirmed on test nodes that restart issue no longer occurs

Signed-off-by: Jason Frame <jason.frame@consensys.net>

matkt · 2024-02-05T15:30:12Z

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

+
+  public static boolean isCodeHashValue(final byte[] key, final byte[] value) {
+    final Hash valueHash = Hash.hash(Bytes.wrap(value));
+    return Bytes.wrap(key).equals(valueHash);


there is no risk to find a key value that match for codeByHash but it's codeByAddress in reality ?

Don't think this can happen in reality. To do that would need to be able to predict the address and have that as the runtime code. I've gone through the scenarios for create and create2 and I can't see a case where this could happen. In the case of create it's not possible to predict the address since it hashes the nonce and sender address and applies the etherum address rule. In the case of the create2 it is more interesting since we can control the inputs, but I still don't think it's possible since you would need the initCode to create a contract with the runtime code of the address which is not possible to predict in advance although we can calculate it in the case of create2.

Also would prefer to replace this detection with the database metadata approach once that is available.

do we know when it will be available ?

Not sure. Think it's still a work in progress https://github.com/fab-10/besu/tree/database-metadata-refactor. So might be a little while off.

matkt

is it working fine with a call to resyncWorldstate API RPC ? should be fine just to be sure

jframe · 2024-02-06T10:16:53Z

is it working fine with a call to resyncWorldstate API RPC ? should be fine just to be sure

Tested with resyncWorldState API RPC and it's working fine

matkt

Looking at the tests and the code seems good for me

Signed-off-by: Jason Frame <jason.frame@consensys.net>

…h_v2

jframe added 2 commits January 31, 2024 17:21

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 <jason.frame@consensys.net>

Fix codehash detection

625a0d4

Signed-off-by: Jason Frame <jason.frame@consensys.net>

jframe marked this pull request as ready for review February 2, 2024 00:05

siladu added the TeamGroot GH issues worked on by Groot Team label Feb 2, 2024

jframe requested review from matkt and garyschulte February 2, 2024 00:41

jframe added 2 commits February 2, 2024 12:07

remove prefix on code hash storage

7c62202

Signed-off-by: Jason Frame <jason.frame@consensys.net>

Merge branch 'main' into code_storage_by_hash_v2

8f8ff25

matkt reviewed Feb 5, 2024

View reviewed changes

matkt approved these changes Feb 6, 2024

View reviewed changes

jframe added 3 commits February 7, 2024 10:50

add changelog entry

47fad26

Signed-off-by: Jason Frame <jason.frame@consensys.net>

Merge remote-tracking branch 'upstream/main' into code_storage_by_has…

827b076

…h_v2

Merge branch 'main' into code_storage_by_hash_v2

bd12fd8

jframe enabled auto-merge (squash) February 7, 2024 01:33

jframe merged commit 2ae6c73 into hyperledger:main Feb 7, 2024
51 checks passed

jframe deleted the code_storage_by_hash_v2 branch February 7, 2024 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code storage by hash v2 #6505

Code storage by hash v2 #6505

jframe commented Jan 31, 2024

github-actions bot commented Jan 31, 2024 •

edited by jframe

Loading

fab-10 commented Jan 31, 2024

jframe commented Jan 31, 2024 •

edited

Loading

jframe commented Feb 2, 2024

matkt Feb 5, 2024

jframe Feb 6, 2024

matkt Feb 6, 2024

jframe Feb 6, 2024

matkt left a comment •

edited

Loading

jframe commented Feb 6, 2024

matkt left a comment

Code storage by hash v2 #6505

Code storage by hash v2 #6505

Conversation

jframe commented Jan 31, 2024

PR description

github-actions bot commented Jan 31, 2024 • edited by jframe Loading

fab-10 commented Jan 31, 2024

jframe commented Jan 31, 2024 • edited Loading

jframe commented Feb 2, 2024

matkt Feb 5, 2024

Choose a reason for hiding this comment

jframe Feb 6, 2024

Choose a reason for hiding this comment

matkt Feb 6, 2024

Choose a reason for hiding this comment

jframe Feb 6, 2024

Choose a reason for hiding this comment

matkt left a comment • edited Loading

Choose a reason for hiding this comment

jframe commented Feb 6, 2024

matkt left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 31, 2024 •

edited by jframe

Loading

jframe commented Jan 31, 2024 •

edited

Loading

matkt left a comment •

edited

Loading