Core/v2 #824
Core/v2 #824
Conversation
mooselumph
requested review from
dmanc,
cody-littley,
hopeyen,
bxue-l2,
jianoaix and
ian-shim
|
|
||
| operatorCounts := []uint{4} | ||
|
|
||
| numBlob := 1 // must be greater than 0 |
There was a problem hiding this comment.
comment from @hopeyen: could we add more tests such as different number of blobs and blobs that would be considered invalid by the verifier?
| NumChunks uint32 | ||
| } | ||
|
|
||
| func (p BlobVersionParameters) MaxNumOperators() uint32 { |
There was a problem hiding this comment.
This is theoretical max number of operators based on the config, which is different from the max number of operators imposed on contracts. Do we need this method or just use what's on the contracts?
It's problematic if a BlobVersionParameters is set in a way that allows max # operators smaller than what's configured onchain, so it looks like we already have to make sure the set of parameters allow the max # operators configured onchain when they're added
There was a problem hiding this comment.
Good callout. Since all of this will be on chain, what about having some on-chain checks to ensure consistency when any of these parameters are updated?
There was a problem hiding this comment.
That's a good idea. Can you add a quick comment over BlobVersionParameters that this is supposed to go onchain?
cc @0x0aa0
core/v2/assignment.go
Outdated
| @@ -0,0 +1,123 @@ | |||
| package corev2 | |||
There was a problem hiding this comment.
should we just name the package v2?
core/v2/utils.go
Outdated
| @@ -0,0 +1,26 @@ | |||
| package corev2 | |||
There was a problem hiding this comment.
should we replace existing usages?
| lengthProofChan := make(chan LengthProofResult, 1) | ||
| commitmentChan := make(chan CommitmentResult, 1) | ||
| proofChan := make(chan ProofsResult, 1) | ||
| commitmentsChan := make(chan commitmentsResult, 1) |
There was a problem hiding this comment.
Just making sure, the changes here are just refactors to allow us to expose GetFrames and GetCommitments. None of the functionality changes in any major way?
Why are these changes needed?
v2 of core library
Checks