-
Notifications
You must be signed in to change notification settings - Fork 201
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
Extract blob key computation logic into static function #1179
Conversation
Signed-off-by: litt3 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fairly straight forward change, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idea LGTM, but not a big fan of having a seemingly function that takes a bunch of arguments whose relationship is unclear. I mentioned this already in arch's onchain renaming PR but I would prefer if we had a struct with a name for "DACertWithHashedPaymentHeader". Then this function could be a method on that struct.
If we're opening the conversation for more fundamental changes, my question is: It seems over complicated to define multiple versions of the same object with subtle differences. |
@samlaf As suggested by @mooselumph in slack, let's go with this simple static function for now. The other options will require a longer discussion, and much more code changes to complete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. let's roll with this for now
Why are these changes needed?
BlobHeader
with the data available on chain, since we don't have access to the payment metadataBlobHeader
middle manEigenDACertificate
, which doesn't contain payment metadataChecks