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

Added logic to extract/validate MPT proof paths. #2989

Conversation

HristoStaykov
Copy link
Contributor

We have added logic to extract proof paths from our MPT implementation and to validate that a previously extracted proof path reaches a giver root hash. A unit test is added that fills an MPT and then validates the paths extracted for every key/value inserted.

  • Problem Overview
    Completed implementation of extraction/verification of proof paths from our MPT implementation.
  • Testing Done
    A new unit test was added that fills a MPT and then every inserted key/value is checked.

kvbc/test/sparse_merkle/tree_test.cpp Show resolved Hide resolved
kvbc/include/sparse_merkle/internal_node.h Outdated Show resolved Hide resolved
kvbc/include/sparse_merkle/base_types.h Show resolved Hide resolved
constexpr size_t level_0_start = MAX_CHILDREN / 2;
return level_0_start + nibble.data();
}

size_t BatchedInternalNode::height(size_t index) const {
size_t BatchedInternalNode::height(size_t index) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to document this strange function ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is documented in the .h file, basically we pass the index of a child from the array children_, containing the 31 elements, and the return value is the height in the sub-tree. internal_node.h:328

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is one of the functions that I changed to static, because they define the structure of the batched internal node and don't need to access the state of a particular instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

It still does not explain the numbers below:

kvbc/src/sparse_merkle/proof_path_processor.cpp Outdated Show resolved Hide resolved
kvbc/src/sparse_merkle/proof_path_processor.cpp Outdated Show resolved Hide resolved
@cloudnoize cloudnoize force-pushed the storage/changes_for_eth_support branch from fe408d5 to cc3f7db Compare April 13, 2023 10:18
@HristoStaykov HristoStaykov force-pushed the proof_paths_from_mpt_processing branch from ae461ec to dc2a0d9 Compare April 13, 2023 10:39
@HristoStaykov HristoStaykov force-pushed the proof_paths_from_mpt_processing branch from cd02c10 to 7fa9981 Compare April 13, 2023 16:33
@cloudnoize cloudnoize force-pushed the storage/changes_for_eth_support branch from f2bca98 to fea5129 Compare April 24, 2023 10:10
@HristoStaykov HristoStaykov force-pushed the proof_paths_from_mpt_processing branch from 9cfe3c9 to bf8af54 Compare April 24, 2023 12:53
We have added logic to extract proof paths from our MPT
implementation and to validate that a previously extracted
proof path reaches a giver root hash. A unit test is
added that fills an MPT and then validates the paths
extracted for every key/value inserted.
…umber of children.

In verifyProofPath added condition for early termination of directions calculation.
@HristoStaykov HristoStaykov force-pushed the proof_paths_from_mpt_processing branch from bf8af54 to f821232 Compare April 24, 2023 13:53
for (size_t nibble_index = 0; nibble_index < Hash::MAX_NIBBLES; nibble_index++) {
auto current_nibble = keyHash.getNibble(nibble_index).data();
auto node_index = BatchedInternalNode::nibbleToIndex(current_nibble);
size_t foundElemens = 0;
subPath.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

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 clearing is needed, because we reuse the same container when we iterate to extract the directions based on the nibbles of the key for each sub-record of the tree.

@@ -109,7 +113,7 @@ std::vector<Hash> getProofPath(Sliver key, std::shared_ptr<IDBReader> db, const
auto child = node.children()[current_index];
nextNodetype = Nodetype::None;
bool firstInternalNodeFount = false;
size_t foundElemens = 0;
hashesCollectedFromInternalNode.clear();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above, but here we actually go through multiple BatchedInternalNodes to collect the necessary hashes from each one and for all iterations we reuse hashesCollectedFromInternalNode.

@HristoStaykov HristoStaykov merged commit 8682058 into vmware:storage/changes_for_eth_support Apr 24, 2023
4 checks passed
@@ -97,6 +97,11 @@ std::ostream& operator<<(std::ostream& s, std::optional<T> const& v) {
return s;
}

static constexpr size_t BatchedInternalNodeGetMaxHeight(const size_t children) {
if (children == 0) return 0;
return 1 + BatchedInternalNodeGetMaxHeight(children / 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

You do know that this is just a log2, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is only called during compile time and the max number of children in 1 record is currently 31. We don't expect it to go to values that can actually cause slow down in compilation.

cloudnoize pushed a commit that referenced this pull request Apr 27, 2023
* Added logic to extract/validate MPT proof paths.

We have added logic to extract proof paths from our MPT
implementation and to validate that a previously extracted
proof path reaches a giver root hash. A unit test is
added that fills an MPT and then validates the paths
extracted for every key/value inserted.

* Const correctness additions.

* Fix for arguments ordering.

* Added documentation comments.

* Reduce allocations for better performance.

* Added constexpr function to calculate the number of levels from the number of children.

In verifyProofPath added condition for early termination of directions calculation.

* Fix gcc build errors.

* Changed vectors to arrays for subPath and hashesCollectedFromInternalNode.

* Convert all containers used to boost::static_vector.
mdarade pushed a commit to mdarade/concord-bft that referenced this pull request Apr 28, 2023
* Added logic to extract/validate MPT proof paths.

We have added logic to extract proof paths from our MPT
implementation and to validate that a previously extracted
proof path reaches a giver root hash. A unit test is
added that fills an MPT and then validates the paths
extracted for every key/value inserted.

* Const correctness additions.

* Fix for arguments ordering.

* Added documentation comments.

* Reduce allocations for better performance.

* Added constexpr function to calculate the number of levels from the number of children.

In verifyProofPath added condition for early termination of directions calculation.

* Fix gcc build errors.

* Changed vectors to arrays for subPath and hashesCollectedFromInternalNode.

* Convert all containers used to boost::static_vector.
mdarade pushed a commit to mdarade/concord-bft that referenced this pull request Apr 30, 2023
* Added logic to extract/validate MPT proof paths.

We have added logic to extract proof paths from our MPT
implementation and to validate that a previously extracted
proof path reaches a giver root hash. A unit test is
added that fills an MPT and then validates the paths
extracted for every key/value inserted.

* Const correctness additions.

* Fix for arguments ordering.

* Added documentation comments.

* Reduce allocations for better performance.

* Added constexpr function to calculate the number of levels from the number of children.

In verifyProofPath added condition for early termination of directions calculation.

* Fix gcc build errors.

* Changed vectors to arrays for subPath and hashesCollectedFromInternalNode.

* Convert all containers used to boost::static_vector.
mdarade pushed a commit to mdarade/concord-bft that referenced this pull request May 1, 2023
* Added logic to extract/validate MPT proof paths.

We have added logic to extract proof paths from our MPT
implementation and to validate that a previously extracted
proof path reaches a giver root hash. A unit test is
added that fills an MPT and then validates the paths
extracted for every key/value inserted.

* Const correctness additions.

* Fix for arguments ordering.

* Added documentation comments.

* Reduce allocations for better performance.

* Added constexpr function to calculate the number of levels from the number of children.

In verifyProofPath added condition for early termination of directions calculation.

* Fix gcc build errors.

* Changed vectors to arrays for subPath and hashesCollectedFromInternalNode.

* Convert all containers used to boost::static_vector.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants