feat: fork collator-selection and use escrow for bonding #688
Conversation
|
Any chance you can split up your pr in at least 2 separate commits - one where you just add unchanged forked pallet, and one with the changes you made to it? The current pr with a single commit is quite hard to review |
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
gregdhill
force-pushed
the
feat/collator-escrow
branch
2 times, most recently
from
0d45135
to
500409b
Compare
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
gregdhill
force-pushed
the
feat/collator-escrow
branch
from
500409b
to
718a819
Compare
There was a problem hiding this comment.
The code LGTM, but I do have a concern regarding migrations. Can you check if we need any? At first I thought we'd be fine as long as you forked the exact version that we are currently using, but then I realized that it may actually be a bit more complicated to switch over. I don't know the details of this pallet to be honest, but as I understand it the existing collators have locked KINT/INTR. Wouldn't switching over without migration lead to their KINT/INTR being locked forever? And wouldn't it lead to problems when existing collators quit, as we would attempt to unlock vKINT but didn't lock?
I'd like the above to be addressed. I also think we should give this a test on testnet. I mean, we should test all changes that we make on testnet before deployment, but we should pay particular attention to this one :)
| @@ -427,6 +430,7 @@ pub mod pallet { | |||
| /// Kicks out candidates that did not produce a block in the kick threshold | |||
| /// and refund their deposits. | |||
| pub fn kick_stale_candidates(candidates: Vec<CandidateInfo<T::AccountId, BalanceOf<T>>>) -> Vec<T::AccountId> { | |||
| // TODO: also kick candidates when escrow balance drops | |||
There was a problem hiding this comment.
I don't think there is consensus on whether or not we should, so fine with leaving this as-is for now
This is not a problem because we have not yet enabled registration. |
Signed-off-by: Gregory Hill gregorydhill@outlook.com
Forks
pallet-collator-selectionand distinguishesStakingCurrencyandRewardsCurrencyso that we can use the illiquidEscrowbalance for candidacy bonds.The reason for the split is because the
ReservableCurrencyimplementation on theEscrowpallet is incompatible with the calls made innote_author.