Next round for post #583
Next round for post #583
Conversation
|
|
||
| const POST_CHALLENGE_COUNT: usize = 30; | ||
| const POST_EPOCHS: usize = 3; | ||
| const POST_EPOCHS: usize = 0; | ||
| const POST_PERIODS_COUNT: usize = 1; |
There was a problem hiding this comment.
@nicola need a number here
There was a problem hiding this comment.
@dignifiedquire @nicola - What's the relationship between POST_PERIODS_COUNT and total time to generate a PoSt? I ask this question because PoSt generation takes very little time right now. go-filecoin relies on that fact. If PoSt generation starts to take approximately the whole proving period, we're going to need to change a bunch of software.
| let mut f_in = File::open(sealed_path).expect("invalid path"); | ||
| let mut replica = Vec::new(); | ||
| f_in.read_to_end(&mut replica) | ||
| .expect("failed to read replica"); |
There was a problem hiding this comment.
what is exects?
There was a problem hiding this comment.
It's a misspelling of expects
| trees.push(public_params(bytes).graph.merkle_tree(&replica)?); | ||
| replicas.push(replica); | ||
| } else { | ||
| panic!("faults are not yet supported") |
There was a problem hiding this comment.
Now that you're editing this code... will you think of a way to remove this panic?
There was a problem hiding this comment.
I will work on faults likely tomorrow as a follow up to this and think about handling it then
| { | ||
| let circuit = ZigZagCompound::blank_circuit(&public_params, &internal::ENGINE_PARAMS); | ||
| let _ = ZigZagCompound::get_verifying_key(circuit, &public_params); | ||
| ZigZagCompound::get_verifying_key(circuit, &public_params) | ||
| .expect("failed to generate zigzag verifying key"); |
There was a problem hiding this comment.
You want to use expects here.
There was a problem hiding this comment.
I thought we were going to implement this as a loop driven from the internal API, which takes 'partial' PoSt proofs as inputs and reduces them into an eventual final proof then generates a circuit proof. This still relies on a state-introducing Beacon::get function, but I think we need to respect the abstractions in place and have proofs be pure functions of their public inputs.
My intent isn't to design in review, so this is just context referencing what we discussed before.
I get that this is an incremental update to existing work, and I'm not averse to including it on the path forward — as long as we're still in agreement about the end goal. I do think this version of BeaconPost will probably need to be changed noticeably, so I'd also be okay with just diving into the reorganization first.
If we're on the same page, I commit to spending some time to hammer out the design in a bit more detail, which we can do in a new issue.
If you're now leaning toward trying to drive all the way forward using this approach, let's talk it through some more.
| @@ -39,6 +39,9 @@ impl<T: Domain, V: Vdf<T>> ParameterSetIdentifier for PublicParams<T, V> { | |||
| pub struct PublicInputs<T: Domain> { | |||
| /// The root hashes of the merkle trees of the sealed sectors. | |||
| pub commitments: Vec<T>, | |||
| pub challenge_seed: T, | |||
| // TODO: Actually use the faults once faults are designed. | |||
| pub faults: Vec<u64>, | |||
There was a problem hiding this comment.
Why are faults a public input?
There was a problem hiding this comment.
They need to be public because the blockchain must know they happened.
|
|
||
| for _ in 0..post_periods_count { | ||
| inputs.push(pub_in.challenge_seed.into()); | ||
| inputs.push(compute_root_commitment(&pub_in.commitments).into()); |
There was a problem hiding this comment.
We might be able to use the apex commitment for the implementation of compute_root_commitment. (See #562).
dignifiedquire
force-pushed
the
post-step2
branch
from
8bb54f5
to
994cee5
Compare
|
closing, will open a new PR once this is ready |
post_epochs=1Note: Before this can be merged params need to be regenerated, but waiting to do so until everything is finalized.