Skip to content

Conversation

@rmoreliovlabs
Copy link
Contributor

@rmoreliovlabs rmoreliovlabs commented Jun 27, 2025

Description

Add tmp trie store implementation in snapshot processor

For validating if the trie structure is correct within the Temporary Datasource:

  1. We need to confirm every node exists in the trie.
  2. We need to make sure every children referenced from the parent node exists in the DB.
  3. Sinc all messages are coming in chunks, we should consider that not all children might be in the same chunk. So if you don’t find a child in a chunk, we should check if we can find it in the DB.
  4. If we don’t find a hash in the trie or the DB, then we can say the trie is invalid.
  5. If all the checks are fine, then we move the node to the Actual Trie and delete it from the Temporary Datasource

When the validation is successful, we can move the content from the Temporary Datasource to the actual Trie Store. We need to make sure the temporary entries are deleted after data is moved.

Motivation and Context

Considering the snap sync protocol working theory, we will use a Temporary Trie Store to save the snap sync chunk messages and validate it. One we the validation succeeds , we can move all the data from the Temporary Datasource to the Actual Trie Store

In this PR we are adding the changes to store snap chunks in temporarydatasource and when the snap sync finishes we move it to the Actual Trie Store and clean the Temporary Datasource.

How Has This Been Tested?

Screenshot 2025-06-26 at 1 23 53 PM Screenshot 2025-07-04 at 5 02 20 PM Screenshot 2025-07-04 at 5 00 47 PM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@github-actions
Copy link

github-actions bot commented Jun 27, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@rmoreliovlabs rmoreliovlabs force-pushed the fix-snap-010-impl branch 29 times, most recently from c8c3934 to 7699791 Compare July 2, 2025 03:47
@rmoreliovlabs rmoreliovlabs force-pushed the fix-snap-010-impl branch 2 times, most recently from e8e272c to 9ebc2e3 Compare July 4, 2025 20:56
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 55%)

See analysis details on SonarQube Cloud

@rmoreliovlabs rmoreliovlabs force-pushed the fix-snap-010-impl branch 8 times, most recently from f3fa8f1 to c3bc1d2 Compare July 6, 2025 04:36
Copy link
Collaborator

@fmacleal fmacleal left a comment

Choose a reason for hiding this comment

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

Good job!

I just have a few comments and questions.

private final TrieStore trieStore;
private final KeyValueDataSource tmpSnapSyncKeyValueDataSource;
public static final int TMP_NODES_SIZE_KEY = -1;
private static final int TMP_NODES_SIZE_KEY = -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

question:

why this value is -1? What is this value used for? Is it a seed? If it's a seed, wouldn't be better to be a random value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where the size of the nodes are stored, since we are storing things in a key / value map, we don't have the size of the nodes and RocksDB might create other keys in it's inner workings. We want that size to iterate all the nodes in the map because the key is the index of the node as the nodes were in a list

Copy link
Collaborator

Choose a reason for hiding this comment

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

But if it's a key value map, why not have it set as a random final value instead of a fix -1? The key is fixed, if a new key clashes with this one, it would fail in RocksDB instead of overwrite no?

The -1 it's predictable, maybe it can easily be changed by someone trying to exploit this no?
If I understood, this is the key where you will store the size of all nodes saved, is it?

}
nodes.get(0).setLeftHash(firstNodeLeftHash);

tmpSnapSyncKeyValueDataSource.put(ByteUtil.intToBytes(TMP_NODES_SIZE_KEY), ByteUtil.intToBytes(nodes.size() + existingNodesSize));
Copy link
Collaborator

Choose a reason for hiding this comment

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

question:

What are you saving at this part? After you save all the tries you save the size of the nodes you have in the for plus the size of nodes you have saved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am storing the size of the nodes when every chunk arrives. Each chunk comes with a new list of nodes to be added, so we need to keep the size of the nodes up to date

@rmoreliovlabs rmoreliovlabs force-pushed the add-tmp-trie-store-in-rsk-context branch from a4e52ae to b4cd299 Compare July 17, 2025 16:17
@rmoreliovlabs rmoreliovlabs force-pushed the fix-snap-010-impl branch 2 times, most recently from 43cc2e0 to d683186 Compare July 17, 2025 16:45
@rmoreliovlabs rmoreliovlabs requested a review from fmacleal July 17, 2025 16:45
@rmoreliovlabs rmoreliovlabs force-pushed the add-tmp-trie-store-in-rsk-context branch 7 times, most recently from e1ad2bc to 3e440b6 Compare July 21, 2025 15:36
@rmoreliovlabs rmoreliovlabs force-pushed the add-tmp-trie-store-in-rsk-context branch from 3e440b6 to 00738fb Compare July 21, 2025 17:30
@rmoreliovlabs rmoreliovlabs force-pushed the add-tmp-trie-store-in-rsk-context branch from 00738fb to 4d43533 Compare July 21, 2025 18:36
@sonarqubecloud
Copy link

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.

3 participants