Support parathread registration and collator assignment #393
Conversation
tmpolaczyk
force-pushed
the
tomasz-register-parathread
branch
from
59c7447 to
b4e5d61
Compare
| for para_id in paras { | ||
| // TODO: sweet O(n) db reads | ||
| if let Some(parathread_params) = ParathreadParams::<T>::get(¶_id) { |
There was a problem hiding this comment.
Here we need some way to distinguish parathreads from parachains. I am using the ParathreadParams storage map, but that will do N reads. Here are some alternatives, not sure what's the best one because I don't yet know if params will be used on-chain or not. They will probably be updated over time, so that's why I initially choose the StorageMap. Alternatives:
* StorageMap<ParaId, Params> (current)
* StorageValue<BTreeMap<ParaId, Params>> (all under one key)
* StorageValue<Vec<ParaId>> and StorageMap<ParaId, Params> (combination of both)
* Refactor currentContainerChains from `Vec<ParaId>` to `Vec<(ParaId, IsParathread)>`
There was a problem hiding this comment.
Ideally we should not need another storage item for this. It is very likely that every time we read RegisteredParchains we will need to know whether they are a Parachain or a Parathread, so we might just want to store a struct indicating this already in RegisteredParchains...
There was a problem hiding this comment.
We decided to keep the current StorageMap, but adding a PendingParams = StorageValue<Vec<(ParaId, Params)>> so that changes are only reflected in the next session.
We also would like to change RegisteredParchains into a StorageValue<Vec<(ParaId, Option<Params>)>>, where Params includes both Option<ParathreadParams> and also other things such as paused: bool. But that looks like a big change so it will be done in a future PR.
Coverage Report@@ Coverage Diff @@
## master tomasz-register-parathread +/- ##
==============================================================
- Coverage 76.90% 76.89% -0.01%
Files 105 105
+ Lines 26036 26316 +280
==============================================================
+ Hits 20021 20234 +213
+ Misses 6015 6082 +67
|
| @@ -131,7 +131,7 @@ impl HostConfiguration { | |||
| if self.max_orchestrator_collators < self.min_orchestrator_collators { | |||
| return Err(InconsistentError::MaxCollatorsLowerThanMinCollators); | |||
| } | |||
| if self.collators_per_parathread != 1 || self.parathreads_per_collator != 1 { | |||
| if self.parathreads_per_collator != 1 { | |||
There was a problem hiding this comment.
here we are finally assuming that we always have one collator per parathread right?
There was a problem hiding this comment.
No, here we allow more than one collator per parathread (just like with parachains), but we don't allow one collator to be assigned to more than one parathread (at least not yet).
There was a problem hiding this comment.
I think the layout looks good @tmpolaczyk good job! Now we only need to test it well
tmpolaczyk
added
B7-runtimenoteworthy
There was a problem hiding this comment.
I think it looks good! A couple of tests that I have not seen (maybe I missed them?)
-
A test where we change the parameters but we dont deregister. The params should get update after 2 sessions right?
-
An additional typescript test where we register a parathread (I would rather have redundancy on tests)
Good job!
Adds a new extrinsic
register_parathread, identical toregisterbut allowing some parathread-specific params.Currently the only param is the slot duration, which is not implemented in the client side, but it will allow parathreads to specify how often do they want blocks to be produced.