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

Feature/etcm 533 non membership proof #899

Merged
merged 10 commits into from
Jan 27, 2021

Conversation

bsuieric
Copy link
Contributor

Proofs for non existent values(keys)

In case an address or storage-value does not exist, the proof needs to provide enough data to verify this fact. This means the client needs to follow the path from the root node and deliver until the last matching node. If the last matching node is a branch, the proof value in the node must be an empty one. In case of leaf-type, it must be pointing to a different relative-path in order to proof that the requested path does not exist.

Specification: https://eips.ethereum.org/EIPS/eip-1186
Jira ticket: https://jira.iohk.io/projects/ETCM/issues/ETCM-533

Proposed Solution
following similar implementation with geth client: in case of non existing account, an error should be returned. In case of non existing storage keys, the proof should be returned with default value 0

Manual tests based on other clients are welcomed

Copy link
Contributor

@dmitry-worker dmitry-worker left a comment

Choose a reason for hiding this comment

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

major:
Please add some tests with negative scenarios! (when the proof is not found for example)

minor:
As soon as we have

  case class StorageProof(
      key: StorageProofKey,
      value: BigInt,
      proof: Seq[ByteString]
  )

we can probably replace the resulting tuples in some of the methods with a similar structure, that can be upgraded to a StorageProof when storage key is available. StorageProof can also inherit the Proof trait and represent it without copying, if needed.

@bsuieric bsuieric requested a review from 1015bit January 22, 2021 11:48
@dzajkowski
Copy link
Contributor

@bsuieric looks like it's not formatted

@AnastasiiaL
Copy link
Contributor

Nice addition with tests for MPT and making a proof always return a root node! Now we are following the spec, but with this we part ways with geth, which returns an empty proof instead of Seq(rootNode). Can you check if besu does the same as geth? We need to make a decision here - to follow the spec or other clients implementation.

@bsuieric bsuieric force-pushed the feature/ETCM-533-non-membership-proof branch from 6699c2a to 5f29089 Compare January 26, 2021 09:28
@bsuieric bsuieric force-pushed the feature/ETCM-533-non-membership-proof branch from 5f29089 to 4ba7d45 Compare January 26, 2021 09:51
@bsuieric bsuieric force-pushed the feature/ETCM-533-non-membership-proof branch from 1eb62b5 to e11992b Compare January 26, 2021 15:56
@bsuieric bsuieric force-pushed the feature/ETCM-533-non-membership-proof branch from e11992b to 72b6a87 Compare January 26, 2021 16:54
@bsuieric bsuieric merged commit 10f1612 into develop Jan 27, 2021
@drospa drospa linked an issue Feb 10, 2021 that may be closed by this pull request
@AnastasiiaL AnastasiiaL deleted the feature/ETCM-533-non-membership-proof branch March 29, 2021 13:31
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.

Implement (non-) membership proof for an MPT element with a given key
5 participants