Make access event keys eip 6800 compatible #7594
Conversation
lu-pinto
force-pushed
the
make-access-event-keys-eip-6800-compatible
branch
from
a12ee48
to
7a26729
Compare
Signed-off-by: Luis Pinto <[email protected]>
Signed-off-by: Luis Pinto <[email protected]>
Signed-off-by: Luis Pinto <[email protected]>
lu-pinto
force-pushed
the
make-access-event-keys-eip-6800-compatible
branch
from
7a26729
to
755f58c
Compare
| } | ||
| } | ||
| if (accountUpdate.isUnchanged()) { | ||
| return; |
There was a problem hiding this comment.
we cannot use if, else if , else if ? I think it will be better
| } | ||
|
|
||
| private void updateAccountStorageState( | ||
| private void generateStorageValue( |
| @@ -40,22 +40,16 @@ public class TrieKeyPreloader { | |||
| public TrieKeyPreloader() { | |||
| this.hasher = new PedersenHasher(); | |||
| trieKeyAdapter = new TrieKeyBatchAdapter(hasher); | |||
| trieKeyAdapter.versionKey( | |||
| Address.ZERO); // TODO REMOVE is just to preload the native library for performance check | |||
| } | |||
|
|
|||
| public List<Bytes32> generateAccountKeyIds() { | |||
There was a problem hiding this comment.
we still need a list for this one ?
lu-pinto
force-pushed
the
make-access-event-keys-eip-6800-compatible
branch
from
657a18e
to
764ce3a
Compare
| private Hash storageRoot; // TODO REMOVE AS USELESS | ||
| private int hashCode; |
There was a problem hiding this comment.
should we remove storageRoot while we are here
There was a problem hiding this comment.
good shout. @matkt mentioned he will remove it in future PR so I didn't want to create conflicts unnecessarily
| .getStorageKeyValuesForUpdate() | ||
| .forEach( | ||
| (storageSlotKey, pair) -> { | ||
| var storageAccountUpdate = worldStateUpdater.getStorageToUpdate().get(accountKey); |
There was a problem hiding this comment.
I don't think this one is still needed as we already checked here https://github.com/hyperledger/besu/pull/7594/files#diff-a8c0068954d44abe33b85a9a6c2b70309adb959255e417350abe54c858574b62R267 but maybe I missing something ?
There was a problem hiding this comment.
Not sure I understand. So the link you posted is when the key values are generated. Here we commit them to the trie in one go.
On generation we only have the storageSlotKey and corresponding key/value. We don't have the storageUpdate ref where we should save the prior value
| final Bytes priorValue = | ||
| accountUpdate.getPrior() == null ? null : accountUpdate.getPrior().serializeAccount(); | ||
| final Bytes accountValue = accountUpdate.getUpdated().serializeAccount(); | ||
| if (!accountValue.equals(priorValue)) { |
There was a problem hiding this comment.
do we still need this check if we have now https://github.com/hyperledger/besu/pull/7594/files#diff-a8c0068954d44abe33b85a9a6c2b70309adb959255e417350abe54c858574b62R208 ?
|
I noticed that adding |
minor change and method renames Signed-off-by: Luis Pinto <[email protected]>
use 0.0.3-SNAPSHOT with timestamp Signed-off-by: Luis Pinto <[email protected]>
lu-pinto
force-pushed
the
make-access-event-keys-eip-6800-compatible
branch
from
2d12b65
to
1a07247
Compare
PR description
This PR encodes balance, nonce, version and code size into a single leaf in Verkle, while code hash has its own leaf, as mandated by EIP-6800.
I'm not doing fixes to gas costs for EIP-4762 yet, most changes are just to superficially not charge for the same leaf twice and to make code compile. I will implement EIP-4762 gas costs in a subsequent PR.
Fixed Issue(s)
This is needed for fixing gas costs for EIP-4762
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests