Conversation
… parameters change
…n (Timekeeper is not updated yet)
…oT chain on it if necessary
…alue as an explicit argument
…od `PotState::update` with no logic changes
…g atomic operations
nazar-pc
requested review from
NingLin-P,
dariolina,
rahulksnv,
shamil-gadelshin and
vedhavyas
dariolina
left a comment
dariolina
left a comment
There was a problem hiding this comment.
Postponing iteration change to the closest entropy injection point is ok. Injection is quite frequent and iteration adjustment will be quite rare, even if/when we make it automatic.
Are checkpoints not included until it's possible to include them in justifications or are you planning to not include them ever?
This, once we update Substrate to a fork that includes paritytech/polkadot-sdk#1211 I'll add them and add corresponding logic. Logic will be a bit different from the current implmentation because we'll be able to check more in verification rather than block import, but both code paths would still have to exist, so current implementation is still useful. Optimistic verification for sync from genesis will be done at some point in the future. |
shamil-gadelshin
left a comment
shamil-gadelshin
left a comment
There was a problem hiding this comment.
"Feature-gated" code is hard to follow and it makes sense to join such blocks where possible.
| } | ||
|
|
||
| #[cfg(feature = "pot")] | ||
| let slot_iterations; |
There was a problem hiding this comment.
Can we put these three variables and two following if-statements under the same cfg-block?
There was a problem hiding this comment.
I did that in some cases, here doing let (slot_iterations, pot_seed, next_slot) = {} would be hardly more readable.
| slot = slot + Slot::from(1); | ||
| } | ||
|
|
||
| #[cfg(feature = "pot")] |
There was a problem hiding this comment.
Could be probably joined with the previous cfg-block.
There was a problem hiding this comment.
Then another level of indentation would have to be added and I wanted to avoid that.
nazar-pc
left a comment
nazar-pc
left a comment
There was a problem hiding this comment.
Also fixed one verification bug that I found while testing entropy injection locally
| } | ||
|
|
||
| #[cfg(feature = "pot")] | ||
| let slot_iterations; |
There was a problem hiding this comment.
I did that in some cases, here doing let (slot_iterations, pot_seed, next_slot) = {} would be hardly more readable.
| slot = slot + Slot::from(1); | ||
| } | ||
|
|
||
| #[cfg(feature = "pot")] |
There was a problem hiding this comment.
Then another level of indentation would have to be added and I wanted to avoid that.
…s, improve logging
…fication method documentation
…urce`, rename `PotSource` into `PotSourceWorker`
…ever included in the block
nazar-pc
dismissed stale reviews from rahulksnv, dariolina, and shamil-gadelshin
via
7ed2bd1
|
I found some issues with implementation and fixed them in last commits. Entropy injection PR will be separate, but this should be all that is necessary to support it. I was trying to find a way to not verify PoT in slot worker, but eventually wasn't able to. |
4 participants
This builds on top of #1922 and extends implementation with two basic capabilities:
Parameters change refers to both entropy injection (where seed is derived from both parent proof and blockchain entropy, which is a special case change from default behavior) and change for number of iterations. Both should coincide, so when we will want to update number of iterations, the actual change will be deferred until next entropy injection. Combining them together simplifies implementation and we should not need to update number of iterations very frequently anyway.
PoT sync is achieved in the following way:
When new proof arrives from one of 3 sources (timekeeper, gossip, block import), verifier will be queried for proofs at higher slot until no such proofs can be found. This ensures that we get to the tip when proofs arrive out of order or when we catch-up block sync and have a gap between best proof in the latest block and the oldest block we have received via gossip.
Proof of time state was introduced, which is a small piece of tricky logic manages information about inputs for the upcoming slot proof. Timekeeper will always use this information as the source of truth and will build proofs on top. This in turn means that syncing proofs to the tip updates the state and re-syncs Timekeeper with the rest of the network.
Code finally starts to take the eventual shape.
There is still TODO for low-latency timekeeper sync (when we receive proof that timekeeper is still working on we should start another timekeeper even before proof is verified to make sure we close the gap in case proof ends up being valid).
Also checkpoints are not sent alongside blocks, which in some cases will decrease verification performance.
Also gossip needs to be tuned and punishment for bad proofs (or proofs that don't contribute to canonical PoT chain) must be implemented.
But overall it is in decent shape.
The next major step is to implement entropy injection and then re-sync implementation and what I have in my head with the spec and have a discussion with Dariia.
Code contributor checklist: