cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.26k stars 3.62k forks source link

Consider adding separate governance key for validators to avoid requiring frequent signing with operator key #1860

Closed cwgoes closed 5 years ago

cwgoes commented 6 years ago

Ref https://github.com/cosmos/cosmos-sdk/pull/1859#discussion_r205927328

cc @sunnya97

gamarin2 commented 6 years ago

I feel this significantly increases the computational overhead. We decided it wasn't a feature needed prelaunch a long time ago, any reason we need it prelaunch?

cwgoes commented 6 years ago

I feel this significantly increases the computational overhead. We decided it wasn't a feature needed prelaunch a long time ago, any reason we need it prelaunch?

It's a "prelaunch proposal", hasn't been decided yet (but probably pre-launch if we want to do it, hard to change later). I have two concerns with the current implementation:

gamarin2 commented 6 years ago

A bit unusual that delegators who vote even when their validators don't are still slashed - don't we want to encourage the delegators to vote?

I think it's mainly because it's really easier to do it this way.

If proposals are frequent, validators will need to frequently bring online their owner key (since we don't have a separate voting key) to vote, which otherwise could be kept in cold storage

Yes, both sunny and I want an additional governance key pair, at least for validators. Sunny wants to do it with sub accounts with fee allowance. But I think we could do it without that. It would require validators to fill an other account with coins to pay for fees but that's better than having to vote with owner key. I would support this feature prelaunch.

rigelrozanski commented 6 years ago

I don't support this feature, especially not prelaunch - I think that delegators should not be delegating to validators who are not going to be voting in governance, delegators should be incentivized to move away from those validators, aka they should be getting slashed.

NOT to mention that implementing this makes the slashing logic increasingly complex, and maybe even breaks under certain circumstances (the validator with very-low self-bond) - seems like this just adds a whole new layer of unnecessary headache - I don't want to support this in protocol - if a validator wants to, they could reimburse their validators without it being explicitly in protocol

cwgoes commented 6 years ago

I don't support this feature, especially not prelaunch - I think that delegators should not be delegating to validators who are not going to be voting in governance, delegators should be incentivized to move away from those validators, aka they should be getting slashed.

I agree, but I think that will happen anyways - if delegators don't want to vote every time, they'll need to find a validator who will (otherwise they'll still get slashed). We want everyone to vote, not just validators.

Hard to predict how any of this will play out in practice though.

NOT to mention that implementing this makes the slashing logic increasingly complex, and maybe even breaks under certain circumstances (the validator with very-low self-bond) - seems like this just adds a whole new layer of unnecessary headache - I don't want to support this in protocol - if a validator wants to, they could reimburse their validators without it being explicitly in protocol

It does make the logic more complex - what do you think of implementing a separate governance key pair for validators and keeping the slashing as-is? That addresses the security concern.

rigelrozanski commented 6 years ago

We want everyone to vote, not just validators.

good point, But the system needs at minimum validators to be voting - I think it makes sense for a delegator to get slashed if their validator didn't vote EVEN if that delegator did vote.

what do you think of implementing a separate governance key pair for validators and keeping the slashing as-is?

Are these two separate questions? Keeping the slashing as is makes sense to me (assuming that it's currently slashing the delegators of a non-voting validator). Implementing a separate governance key pair also makes sense to me, but I think that should be a part of a larger key system (postlaunch) which @sunnya97 and I have discussed previously. (key-system = cold storage master key which can change subkeys, and the permissions of those sub-keys for things like governance, Tendermint signing etc. this could mean that a validator could actually have a series of governance keys)

rigelrozanski commented 6 years ago

I'm going to close this issue for now

cwgoes commented 6 years ago

Are these two separate questions?

We could (should?) have both - but having a separate key would mean that validators don't need to bring their operator key online to vote on every proposal.

I think that should be a part of a larger key system (postlaunch) which @sunnya97 and I have discussed previously. (key-system = cold storage master key which can change subkeys, and the permissions of those sub-keys for things like governance, Tendermint signing etc. this could mean that a validator could actually have a series of governance keys)

Agreed; does this conflict with a simple second voting key now?

I'm going to close this issue for now

Why? It's just a proposal - and at least I'm not convinced we've come to a resolution.

The delegator/validator slashing is debatable, but I think requiring validators to bring their operator key (which we want to be cold-storable, since it can e.g. change their signing key) online frequently is a concern worth addressing.

Reopening, but if this is a duplicate of another previous discussion feel free to point me to it.

rigelrozanski commented 6 years ago

Why?

sorry, closed because I thought the logic was pretty straight forward in justifying that delegators must get slashed for their validators not voting because it's required for network safety. - but fine to leave open to continue discussion as you're not yet satisfied.

Have we come to agreement "delegators must get slashed for non-signing validators?" if so then we should either close or re-name this proposal because that was the original train of thought - it seems like there are some new issues which have been opened through this conversation, such as:

but having a separate key would mean that validators don't need to bring their operator key online to vote on every proposal.

Yes, of course we want to have gov. keys, even multiple gov. keys could be cool, definitely better for validators - again I think this is post launch - which was the conclusions of conversations in the past, as we wanted to roll this out with a more comprehensive key system as I explained - BUT maybe it's a good idea to hack it in prelaunch even without a comprehensive key system. (This is a new issue)

does this conflict with a simple second voting key now?

Although we can't have multiple Tendermint voting keys for a single operator address in the current implementation - I don't see why we wouldn't be able to support multiple governance keys - we just need an M-to-1 map for the valid keys in governance.

The delegator/validator slashing is debatable, but I think requiring validators to bring their operator key (which we want to be cold-storable, since it can e.g. change their signing key) online frequently is a concern worth addressing.

So yeah, reiterating, I sustain that we absolutely need to be slashing delegators of non-voting validators - if you think this is problem because of bringing your operating key online frequently - then maybe we should make a priority for adding a governance key - we should update this issue title to reflect this probably

cwgoes commented 6 years ago

Have we come to agreement "delegators must get slashed for non-signing validators?" if so then we should either close or re-name this proposal because that was the original train of thought - it seems like there are some new issues which have been opened through this conversation, such as:

Prelaunch, agreed - long-term not sure - but agreed that we shouldn't worry about delegator-validator nonvoting slashing for now. Renamed the issue.

BUT maybe it's a good idea to hack it in prelaunch even without a comprehensive key system.

The only reason I think this is even worth considering prelaunch is that requiring semi-online validator keys changes the setup we'll require of validators substantially - otherwise they could keep their operator key in cold storage and access it extremely infrequently after initial setup.

I suggest we consider adding a very simple, optional, second pubkey that validators can elect to set (in a similar manner to EditDescription), which would serve to validate votes instead of the operator key if present.

cc @greg-szabo would appreciate your perspective regarding the validator key side

greg-szabo commented 6 years ago

From an operational perspective, validator keys are a great idea. Apart from the security implications (some keys can be stored in cold storage while others - with a limited power - are online), it also allows different operations teams to take on different responsibilities. The "master" key in cold storage should only be accessible by a selected few, while the operating key is more accessible just because a "maintenance team" (server maintenance, ops updates, security updates, etc) is working near it. The "government votes" key can be handed out to the blockchain researchers team with the chief officers that can plan strategies for the company depending on how the network evolves.

This seems a great separation of duties. However, the usability will be dependant on the implementation. It seems to me that parts of governance are still not pinned down (do you get slashed if your validator didn't vote?) so I'm a bit worried, what other keys, ideas, requirements will surface. Just out of thin air and this discussion, I can think of an key-based ACL system based on the master key that would enable teams to separate all kinds of roles into different keys. But it's hard to see if it's even necessary when I don't see the full picture of governance.

In short: separating the operation key from the voting key sounds like a good idea from my limited perspective.

zmanian commented 6 years ago

I vote for this as a post launch feature.

cwgoes commented 6 years ago

I vote for this as a post launch feature.

OK; seems like the general consensus, let's punt this.

sunnya97 commented 6 years ago

Agreed regarding post-launch.

zmanian commented 5 years ago

One thing I currently think is that a validator operator key must be available online on a semi regular basis anyways to withdraw and bond inflation and fees so it might not make sense to actually separate governance.

it also might make bribing people to hand over their voting authority easier

jackzampolin commented 5 years ago

I think we are using the sub-keys work to satisfy this. Closing in favor of that.

rigelrozanski commented 5 years ago

@jackzampolin please link issues when you're closing them

closing in favour of https://github.com/cosmos/cosmos-sdk/pull/4380