Switch staking Validator.ConsPubkey to Validator.ConsAddr

[rigelrozanski]

Within the fee-distribution I need to retrieve the validator operator address from the Tendermint consensus address - Currently the staking validator keeper holds an index of staking validators by the consensus pubkey but not the consensus address - To achieve what I need I basically need to introduce the ConcAddress and an Index into staking/validator. This said, I'm not sure that we actually need the consensus PubKey at all. Thoughts?

(CC: @ebuchman @cwgoes)

[alexanderbez]

I thought the pubkey is only needed for signing info? Also, can't you do something like sdk.ValAddress(consAddr.Bytes()) to go from TM consensus address to operator address or am I not understanding the problem?

Also, I'll implement #2221 ASAP so there is less of a possibility of conflicting/annoying changes.

[rigelrozanski]

@alexanderbez I think you may have a misunderstanding. ConsAddr is derived from ConsPubKey, OperAddr is derived from OperPubKey. These are two separate keys which cannot be derived from each other should both be held by the validator. - They can of course be found based on each other if the right indices on the validator are setup.

I thought the pubkey is only needed for signing info?

The ConsAddr is needed because the proposer of a block get's slightly more rewards, Tendermint wil pass information of who the proposer is to the sdk in the form of the ConcAddr

Also, I'll implement #2221 ASAP so there is less of a possibility of conflicting/annoying changes.

Much appreciated!

[alexanderbez]

I see. This point has always been of great confusion for me when I updated the bech32 prefixes originally...just went through the init genesis logic and I see the key pair is generated independently of the account (operator) -- thanks!

And I guess to answer your question, I guess we don't need the pubkey?

[rigelrozanski]

Yeah I don't think we do unless I'm missing something - slashing is currently the main consumer of consensus parameters in the SDK

[ValarDragon]

I think only using the consensus address makes a lot of sense. I had a similar conversation with Jae w.r.t. to how we do validator set changes over ABCI. The conclusion was that we had both thought that isolating the state machine from consensus public keys would be a good idea, for efficiency reasons and abstraction reasons. (In tendermint/tendermint#2177 I was advocating for only the pubkey, but Jae made the point that we can just cache the hash calculation, so this is nbd as long as we are only using the address everywhere) This prevents the state machine from ever requiring the validator's consensus key for other things e.g. governance.

I am confused where prefixing comes into play here though. Only the CLI commands should know about the bech32'ing, not the internal state machine.

[cwgoes]

We need the consensus public key to add a new validator in abci.RequestEndBlock.ValidatorUpdates, so we need to store it, but I don't see a reason we would ever need to index by it - indexing by the consensus public key address seems prudent. Can we change our existing by-consensus-pubkey address to by-consensus-address?

[rigelrozanski]

cool so I don't think we need to introduce a new term at all - we can just keep things the way they are but also index by the address instead of the pubkey 👍 .

Roger yes, we need to keep the pubkey stored, just incase an old validator or validator candidate is added to the validator set, at which point it abci.RequestEndBlock.ValidatorUpdates needs it. So until validator updates is changed to use the cons address too we will still need it here. I don't even think that change would even be possible as Tendermint needs the PubKey (not the address) to verify signatures

[rigelrozanski]

@ValarDragon

I am confused where prefixing comes into play here though. Only the CLI commands should know about the bech32'ing, not the internal state machine.

what do you mean? All the different addresses are stored as different types internally (but can be converted on the fly). We do this to simplify the code considerably for displaying address bech

[ValarDragon]

Sorry I meant for the pubkey, should have clarified. That question was resolved by Chris' comment.