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

receipt inclusion proof #25

Merged
merged 48 commits into from
Nov 4, 2024
Merged

Conversation

pedrohba1
Copy link
Contributor

@pedrohba1 pedrohba1 commented Oct 23, 2024

BACK-20

This was reviewed previously in this PR from Forrestrie

What does it all do?

It covers the basic receipt proofs we need for the execution layer data, using the data extracted from firehose.
To be more precise, from this diagram of nozzle, it contains the functions defined in the diagram:

  • receipts_trie(Header,Receipts)
  • merkle_proof(Trie, Receipt)
  • Verify(Log, Header, Receipt)

Copy link

linear bot commented Oct 23, 2024

@suchapalaver suchapalaver self-requested a review October 23, 2024 19:22
Copy link
Contributor

@suchapalaver suchapalaver left a comment

Choose a reason for hiding this comment

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

The work looks great but I think we want to start following the forrestrie-examples pattern for examples like the test here.

Make sure to rebase commits so there are no typos!

crates/forrestrie/src/execution_block.rs Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_block.rs Outdated Show resolved Hide resolved
@pedrohba1 pedrohba1 changed the title Pedro/back 20 receipt inclusion proof receipt inclusion proof Oct 25, 2024
@pedrohba1 pedrohba1 self-assigned this Oct 25, 2024
@pedrohba1 pedrohba1 force-pushed the pedro/back-20-receipt-inclusion-proof branch 3 times, most recently from 7ed8122 to f79d611 Compare November 1, 2024 16:58
@pedrohba1 pedrohba1 force-pushed the pedro/back-20-receipt-inclusion-proof branch from c933454 to 3afe47b Compare November 1, 2024 17:12
@pedrohba1 pedrohba1 force-pushed the pedro/back-20-receipt-inclusion-proof branch from 3afe47b to 6badc7f Compare November 1, 2024 17:25
@pedrohba1
Copy link
Contributor Author

pedrohba1 commented Nov 1, 2024

The work looks great but I think we want to start following the forrestrie-examples pattern for examples like the test here.

Make sure to rebase commits so there are no typos!

I'm adding the example into another PR this is another task that relies on this.

crates/forrestrie/src/execution_layer.rs Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_layer.rs Outdated Show resolved Hide resolved
crates/forrestrie-examples/examples/receipts_proof.bak Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_layer.rs Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_layer.rs Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_layer.rs Outdated Show resolved Hide resolved
crates/forrestrie/src/execution_layer.rs Show resolved Hide resolved
crates/firehose-protos/build.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@suchapalaver suchapalaver left a comment

Choose a reason for hiding this comment

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

Thanks for all the changes!

Couple of wishes for the future:

  • go with creating tickets/issues instead of leaving TODO notes to self in the code.
  • unless they take too long, wait for the authors of comments to resolve them themselves.
  • use git rebase to squash your commits down to a reasonable number. Should 5 or less commits per PR usually. It depends.

@pedrohba1 pedrohba1 merged commit 4b62d16 into main Nov 4, 2024
7 checks passed
@pedrohba1 pedrohba1 deleted the pedro/back-20-receipt-inclusion-proof branch November 4, 2024 16:15
suchapalaver added a commit that referenced this pull request Nov 28, 2024
…-note-on-crates-to-repo-readme

docs(README): add section on workspace crates
suchapalaver pushed a commit that referenced this pull request Nov 28, 2024
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.

2 participants