IPC-304: add permissioned flag to disable validator changes #305
Conversation
|
Fwiw, I don't think this is the sort of thing that would be much better implemented as a compilation flag. It'd be great to have support for those generally, but this approach looks fine. |
| // This means that initial validators won't be able to recover | ||
| // their collateral ever (worth noting in the docs if this ends | ||
| // up sticking around for a while). | ||
| if (s.permissioned) { |
There was a problem hiding this comment.
- Probably I missed the discussion but why are we blocking
leaveas well? - If we decide to do that, shouldn't we enforce it only for the already boostrapped network to keep it symmetrical with
join(maybe not important in practice but easier to reason about IMO)
There was a problem hiding this comment.
If we're limiting membership changes to reduce the changes of things going wrong, it makes sense to limit them both ways and just not have to worry about those paths. Don't see a lot of value in keeping leave as presumably you're not starting with a large validator set that you can afford to lose over time.
There was a problem hiding this comment.
Probably I missed the discussion but why are we blocking leave as well?
We don´t want any validator change for now, as this is what is breaking Mycelium :)
If we decide to do that, shouldn't we enforce it only for the already boostrapped network to keep it symmetrical with join (maybe not important in practice but easier to reason about IMO)
This is a good point, fixed it
| @@ -31,6 +31,7 @@ contract SubnetActorDiamond { | |||
| uint16 activeValidatorsLimit; | |||
| uint256 minCrossMsgFee; | |||
| int8 powerScale; | |||
| bool permissioned; | |||
There was a problem hiding this comment.
Technically, it is not permissioned. Here we do not allow to join even if the validator is "permissioned". I do not know the correct term for that.
There was a problem hiding this comment.
static is probably the accurate term, but not too descriptive. staticMembership ?
There was a problem hiding this comment.
The reason why I went with permissioned is because this is a first stage towards this design for federated validation: https://www.notion.so/pl-strflt/Federated-validation-6290874b58904818ac4fdddb5128d01b
I have no strong opinions, I can name this static until we have the permissioned feature.
…/ipc-solidity-actors into ipc-204-permissioning
Fixes consensus-shipyard/ipc#40
Background
This is a first stage towards adding the option to make subnets permissioned. In this case, I included a
permissionedflag that can be set in construction that determines if validator changes should be disabled or not.Why using a construction flag? Ideally, I would've loved to use a compilation flag, so you can already compile a contract with the subset of features that you want, but Solidity doesn't have support for compilation flags, and there doesn't seem to be an easy way of getting an analogous behavior without a lot of redundant code. This approach also allows us to have this functionality without having to explicitly re-write or break any of our existing code.
In the short-term, I expect validator changes to be disable for Mycelium and other public deployments, while in user child subnets we may want to leave them enabled so users can test the full-functionality of testnets.
Next steps
I wanted to double-check that everyone is OK with this approach before making further progress. If this is the case, the next steps is to:
permissionedscenario.ipcandfendermint(I will open the corresponding issues).