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

V2.2.2 eigenda #6

Open
wants to merge 7 commits into
base: eigenda
Choose a base branch
from
Open

V2.2.2 eigenda #6

wants to merge 7 commits into from

Conversation

renlulu
Copy link

@renlulu renlulu commented Jan 22, 2024

For review purpose, don't merge!

// record preimage data
log.Info("Recording preimage data for EigenDA")
shaDataHash := sha256.New()
shaDataHash.Write(sequencerMsg)

Choose a reason for hiding this comment

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

why are we recording the hash of the sequencer message and not the underlying data?

Copy link
Author

Choose a reason for hiding this comment

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

Oh actually we are recording the underlying data, see line 211. here we are calculating the key of it :D

Copy link

@teddyknox teddyknox Jan 25, 2024

Choose a reason for hiding this comment

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

I see, but it seems like sha256(data) != dataHash on line 211. Right now the dataHash variable is based on the hash of the sequencer message, which itself is a reference the EigenDA blob, not the blob itself.

To put it formally: data != sequencerMsg, which breaks the invariant of the preimage cache necessary for fraud proofs to work as expected.

Copy link
Author

Choose a reason for hiding this comment

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

Yes you are right, here's the map working as the preimage cache, the key of the map I put here is sha256(reference) and value is data itself, and later when i query from the map again (file cmd/replay/main.go), i also hashed data reference, so i can get correct data. But i got you, maybe you prefer using hash(data) as key? i can make the change : D

Copy link
Author

Choose a reason for hiding this comment

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

Oh but i double check PreimageEigenDAReader it has to implement interface EigenDAReader, so the parameter need to be blob reference lol

Choose a reason for hiding this comment

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

Maybe I misunderstand. Isn't the preimage cache for reading data that isn't in the inbox? The blob reference is in the inbox.

* refactor: clean doc

* doc: add todo for replay

* doc: add alt license header

* doc: change some comment for replay
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