-
Notifications
You must be signed in to change notification settings - Fork 782
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
client: add proofs to blobsbundle #2642
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
09f1111
to
aef3d40
Compare
Rebased this via UI |
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.
It would be good to have at least 1-2 small test cases for this so to minimally make sure that data is passed around and encoded/decoded in the correct format.
st.ok(pendingBlob !== undefined && equalsBytes(pendingBlob, blobs[0])) | ||
const blobProof = blobBundle.proofs[0] | ||
st.ok(blobProof !== undefined && equalsBytes(blobProof, proofs[0])) |
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.
@holgerd77 this checks if the bundle has the proof and valdiates it as well,
Also getBlobsBundle api is being removed (discussed in yesterdays 4844 call) and the bundle and payload is going to be included in a new getPayloadV3, so will make sure spec test for that will cover proof check in the bundle
aef3d40
to
2f8c3f8
Compare
rebased via UI again |
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.
LGTM
Add proofs to blobsbundle and consequently to getblobsbundle engine api call so that CL doesn't need to compute proof which ideally would be coming packaged in the network wrapper serialized tx
ref: