PeerDAS: Add KZG verification when sampling #14187
Conversation
| if !verified { | ||
| return pubsub.ValidationReject, errors.New("failed to verify kzg proof of column") | ||
| } | ||
|
|
||
| // ????? |
There was a problem hiding this comment.
@nisdas to what specification rule this portion of code does correspond?
There was a problem hiding this comment.
We are just retrieving the state here, it doesnt correspond to any specification rule directly
There was a problem hiding this comment.
Actually I was more talking about the following block:
if err := coreBlocks.VerifyBlockHeaderSignatureUsingCurrentFork(parentState, ds.SignedBlockHeader); err != nil {
return pubsub.ValidationReject, err
}There was a problem hiding this comment.
[REJECT] The proposer signature of sidecar.signed_block_header, is valid with respect to the block_header.proposer_index pubkey.
nalepae
force-pushed
the
peerdas-check-kzg-sampling
branch
2 times, most recently
from
875ac52
to
3a36a70
Compare
nalepae
force-pushed
the
peerdas-check-kzg-sampling
branch
from
3a36a70
to
330a87e
Compare
nalepae
requested review from
prestonvanloon,
terencechain and
nisdas
and removed request for
a team
| if !verified { | ||
| return pubsub.ValidationReject, errors.New("failed to verify kzg proof of column") | ||
| } | ||
|
|
||
| // ????? |
There was a problem hiding this comment.
We are just retrieving the state here, it doesnt correspond to any specification rule directly
| if err != nil { | ||
| return pubsub.ValidationIgnore, err | ||
| // Add specific debug log. | ||
| if logrus.GetLevel() >= logrus.DebugLevel { |
There was a problem hiding this comment.
I dont think there is any reason to gate it this way as a time calculation is very cheap.
Please read commit by commit.
Before this PR,
VerifyKZGInclusionProofColumnandVerifyDataColumnSidecarKZGProofswere not evaluated when receiving columns from a peer during sampling.Previously, only the root and the column index were evaluated. This allowed a peer to submit a fabricated column with a correct root and index, which posed a security risk.
Now,
VerifyKZGInclusionProofColumnandVerifyDataColumnSidecarKZGProofsare evaluated for every sampled column, and the corresponding columns are rejected if these evaluations fail.