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

core: move RLP encoding of values to trie #25431

Closed
wants to merge 2 commits into from

Conversation

gballet
Copy link
Member

@gballet gballet commented Jul 28, 2022

Move the code that serializes an MPT leaf to the MPT tree code itself.

Rationale

The encoding of the leaf in RLP is very much dependent on the format of the tree, and performing the encoding in the state database is causing an abstraction leak: Verkle trees use a different format, and therefore a distinction as to the type of tree has to be made at the database level, which imo should be abstracted to the tree itself.

Consequence

Instead of writing the following in state_object.go's updateTrie:

                if (value == common.Hash{}) {
                      if tr.IsVerkle() {
                              k := trieUtils.GetTreeKeyStorageSlotWithEvaluatedAddress(s.pointEval, new(uint256.Int).SetBytes(key[:]))
                              s.setError(tr.TryDelete(k))
                      } else {
                              s.setError(tr.TryDelete(key[:]))
                      }
                        s.db.StorageDeleted += 1
              } else {
                      // Encoding []byte cannot fail, ok to ignore the error.
                      v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:]))
                      if !tr.IsVerkle() {
                              s.setError(tr.TryUpdate(key[:], v))
                      } else {
                              // Update the trie, with v as a value
                              s.setError(tr.TryUpdate(key, value[:]))
                      }
             }

the code that supports both MPT and verkle trees become:

                var v []byte
                if (value == common.Hash{}) {
                        s.setError(tr.TryDelete(key[:]))
                        s.db.StorageDeleted += 1
              } else {
                      // Encoding []byte cannot fail, ok to ignore the error.
                      v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:]))
                      s.setError(tr.TryUpdate(key[:], value[:]))
                      s.db.StorageUpdated += 1
              }

It should have no impact on the root hash of the tree (although there is still an issue with the light client, which is why this PR is still a draft)

Note that rlp.EncodeToBytes is now called twice (in the case of an MPT), in case it ends in the snapshot. This pertains to the snapshot format, which ideally should also be abstracted but doesn't belong in this PR.

@gballet gballet added the verkle label Jul 29, 2022
@fjl
Copy link
Contributor

fjl commented Nov 22, 2022

This change was merged as part of a different PR, #25458.

@fjl fjl closed this Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants