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

Add attestation quote to Espresso payload #385

Merged
merged 11 commits into from
Dec 18, 2024

Conversation

Sneh1999
Copy link

@Sneh1999 Sneh1999 commented Dec 14, 2024

This PR:

This PR fixes this issue

Key places to review:

Please review this PR carefully as its a very important PR

@Sneh1999 Sneh1999 force-pushed the add-tee-proof-to-espresso-payload branch from 504da89 to a7c11d2 Compare December 15, 2024 00:57
@Sneh1999 Sneh1999 marked this pull request as ready for review December 15, 2024 23:00
Copy link
Member

@ImJeremyHe ImJeremyHe left a comment

Choose a reason for hiding this comment

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

Looks good. Just some comments.

This PR sends all the messages to espresso, which helps nodes derive L2 state from Espresso. My question is why users not to derive state from seqeuncer inbox, but from Espresso. If this scenario exists, that also means we can't activate the escape hatch, which may skip submitting messages to espresso.

Does finality node need changes according to these changes? I didn't see its modifications in this PR

arbnode/batch_poster.go Show resolved Hide resolved
arbnode/transaction_streamer.go Show resolved Hide resolved
arbnode/transaction_streamer.go Outdated Show resolved Hide resolved
@ImJeremyHe ImJeremyHe self-requested a review December 16, 2024 15:25
Copy link
Member

@zacshowa zacshowa left a comment

Choose a reason for hiding this comment

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

I just had a couple questions related to the nature of the cryptographic processes being used with the attestation quote. If they are moot, this looks good to me.

arbnode/transaction_streamer.go Outdated Show resolved Hide resolved
arbnode/transaction_streamer.go Outdated Show resolved Hide resolved
@Sneh1999
Copy link
Author

Looks good. Just some comments.

This PR sends all the messages to espresso, which helps nodes derive L2 state from Espresso. My question is why users not to derive state from seqeuncer inbox, but from Espresso. If this scenario exists, that also means we can't activate the escape hatch, which may skip submitting messages to espresso.

Does finality node need changes according to these changes? I didn't see its modifications in this PR

I think your concern is valid, escape hatch might not work well with the finality node. We can change the finality node later and think about this later as well. But yes, we should keep this in mind when we are making changes to the finality node

@ImJeremyHe ImJeremyHe force-pushed the add-tee-proof-to-espresso-payload branch 2 times, most recently from e0af17e to 4bf08d7 Compare December 17, 2024 08:29
@ImJeremyHe ImJeremyHe force-pushed the add-tee-proof-to-espresso-payload branch from 4bf08d7 to 16c22a2 Compare December 17, 2024 09:08
@ImJeremyHe ImJeremyHe force-pushed the add-tee-proof-to-espresso-payload branch from fc094b1 to dd3f7ef Compare December 17, 2024 11:25
@ImJeremyHe ImJeremyHe force-pushed the add-tee-proof-to-espresso-payload branch from dd3f7ef to 139ac64 Compare December 17, 2024 12:19
@Sneh1999 Sneh1999 merged commit 47e6010 into celestia-integration Dec 18, 2024
13 checks passed
@Sneh1999 Sneh1999 deleted the add-tee-proof-to-espresso-payload branch December 18, 2024 00:31
ImJeremyHe pushed a commit that referenced this pull request Dec 18, 2024
* Add attestation quote to Espresso payload

* cleanup code

* fix lint

* add position

* fix verify namespace proof bug

* Increase hotshot transaction limit

* build hotshot payload and add unitests for it

* Verify merkle proof first

* Rename

* push minor fixes

---------

Co-authored-by: ImJeremyHe <[email protected]>
(cherry picked from commit 47e6010)
Sneh1999 added a commit that referenced this pull request Dec 18, 2024
* Move tee address (#392)

* move espressoTeeVerifierAddr to transaction streamer

This commit moves the TEE verifier contract address to the transaction
streamer, configurable via the batch posters config.

* Fix tests and transaction_streamer

* use a string for tee verifier address

Koanf cannot parse a common.Address by default, so we should use a
string when taking in this config option.

* fix compilation

* fix e2e test

* Fix config parsing

* Fix lint and run formatter

* Remove outdated test

(cherry picked from commit 30689d6)

* Add attestation quote to Espresso payload (#385)

* Add attestation quote to Espresso payload

* cleanup code

* fix lint

* add position

* fix verify namespace proof bug

* Increase hotshot transaction limit

* build hotshot payload and add unitests for it

* Verify merkle proof first

* Rename

* push minor fixes

---------

Co-authored-by: ImJeremyHe <[email protected]>
(cherry picked from commit 47e6010)

---------

Co-authored-by: Zach Showalter <[email protected]>
Co-authored-by: Sneh Koul <[email protected]>
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