Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Genesis bonds have an owner address in place of a validator address #1869

Closed
4 tasks
amrali opened this issue Jul 29, 2018 · 8 comments
Closed
4 tasks

Genesis bonds have an owner address in place of a validator address #1869

amrali opened this issue Jul 29, 2018 · 8 comments

Comments

@amrali
Copy link

amrali commented Jul 29, 2018

Summary of Bug

In the genesis.json file the bonds list generated by gaiad init or filled out by testnet participants contains two addresses, one for the delegator's address and another for the validator's address.

 "bonds": [
        {
          "delegator_addr": "cosmosaccaddr1y4rdzkwvl3d3qg54fd4uk6zdz34f2rkck4d9jt",
          "validator_addr": "cosmosaccaddr1y4rdzkwvl3d3qg54fd4uk6zdz34f2rkck4d9jt",
          "shares": "100",
          "height": "0"
        }

The validator_addr value is the owner's account address for a compressed Secp256k1 public key. For my validator Ed25519 public key, the address instead should be cosmosvaladdr17zxzlwh0842czgxk7hcj60enwx9368kg452u56.

Why does it work on currently running testnets? It doesn't make sense for an owner account to delegate to itself (it doesn't represent the validator identity, does it?).

Steps to Reproduce

  • gaiad init
  • Examine the generated genesis.json file.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@rigelrozanski
Copy link
Contributor

Pretty sure this is functioning as expected, I'm assuming that this key cosmosvaladdr17zxzlwh0842czgxk7hcj60enwx9368kg452u56 is the address generated from your signing key used by tendermint correct? If this is the case, note that these keys are used exclusively by tendermint and not by the SDK at all. For the SDK the validator owner address is used as the main index, thus the key your seeing in the genesis is actually correct.

Also note that each validator must have a delegation to itself- this is referred to as the self-delegation (or sometimes self-bond) - The accounting mechanisms are identical for a validator's self-bond and other delegations so it makes sense to combine them in this way.

I'm going to close this issue but feel free to still ask questions if my above explanation doesn't clear things up

@amrali
Copy link
Author

amrali commented Jul 30, 2018

@rigelrozanski Yes that's the address for the signing key. So the owner's wallet/key is where the block rewards/provisions will be distributed? If so does that mean that the signing key is truly solely used to sign consensus messages and doesn't have any accounting applied to it?

Also why do we have to specify validator_addr if it will always be the owner's public key address?

@joe-bowman
Copy link
Contributor

joe-bowman commented Jul 30, 2018

An end user, wanting to delegate to a validator, being asked for a validator_addr should expect to use a cosmosValAddr bech32, not an cosmosAccAddr which bears no resemblance to the validator to which they wish to delegate.

It seems completely unituitive to be delegating to the owner's account, and not the validator. The two are separate entities. Even if cosmos-sdk does not concern itself with the validator's address, it should handle this internally without making this the users' problem. UX is a thing that might not matter now, but certainly will when the wider community become involved.

@amrali
Copy link
Author

amrali commented Jul 30, 2018

To further expand on @joe-bowman UX comment. I think the "owner" concept should simply be replaced with a validator and an associated wallet (where all the accounting takes place), so that people just specify their validator address/publickey in configuration files and simply link it with a wallet account either via a TX (e.g., AssociateValidatorAccount) or within genesis.json. Treating a wallet (aka owner) account as the validator identity is quite confusing.

@rigelrozanski
Copy link
Contributor

@amrali

So the owner's wallet/key is where the block rewards/provisions will be distributed?

correct, when they submit a "withdrawal rewards" transaction (in development) this is the default withdrawal location, although they will also be able to specify a new withdrawal address

If so does that mean that the signing key is truly solely used to sign consensus messages and doesn't have any accounting applied to it?

If by signing key you mean Tendermint signing key - then yes

Also why do we have to specify validator_addr if it will always be the owner's public key address?

The bonds are a common struct used for both the validators self-delegation as well as other delegations - thus we use the common struct to represent the self-delegation which is the only type of delegation where those two will be equal - I think abstracting it any further to isolate out for the validator self-delegation case is an over-abstraction.

@joe-bowman

It seems completely unituitive to be delegating to the owner's account, and not the validator. The two are separate entities.

I agree to a degree - however when you send a delegation to the validator, whether it's indexed by the address with Bech valAddr or accAddr you are still sending it to that validator - not their personal account, they can't spend it right - it's held by the protocol - I think this should be made very clear in the UI (CC @faboweb) but yeah like - it's the delegation index for the validator not the actual destination of the delegated funds

I think the "owner" concept should simply be replaced with a validator and an associated wallet

Yeah maybe we should rename from "owner" to something like "trustee" because the validator's creator doesn't actually own the contents of the validator they just operate with it.

Treating a wallet (aka owner) account as the validator identity is quite confusing.

Although on some level I agree with you, this issue has been extensively discussed internally, and the conclusion has been that the best index for the validator must be the "trustee" ("owner") address due to the need to maintain something static as validators may be able to change/rotate through their signing keys which is desirable - this is going to be an ongoing issue, but I'm certain that the tendermint signing-key/address is NOT the correct index to be using. It's possible we may introduce a totally new value, say "validator-index" or "nonce" which is independent of both addresses and is used as the index - but that is certainly post launch

@amrali
Copy link
Author

amrali commented Jul 30, 2018

It's possible we may introduce a totally new value, say "validator-index" or "nonce" which is independent of both addresses and is used as the index - but that is certainly post launch

@rigelrozanski Thanks for answering our questions, this has been very helpful. I somewhat agree with the "validator-index" idea. If we are going to consider this post launch perhaps we should consider just redefining the terminology so that it correctly conveys the underlying meanings.

IOW, I believe a validator's identity should be its account and the validation signing key should just be one of many keys. In fact, this may allow for 1-to-M mapping of <validator account, validation key> which should (I'm not sure) allow a single validator account to use multiple signing keys for multiple validator nodes and the network should only accept either one of the produced signatures for a single validator account.

If possible, this immediately solves high availability by allowing validator node redundancy without any coordination. Which signature/publickey the network should select can either be a priority field or some form of a sorting order.

@rigelrozanski
Copy link
Contributor

If we are going to consider this post launch perhaps we should consider just redefining the terminology so that it correctly conveys the underlying meanings.

agreed

@ebuchman
Copy link
Member

ebuchman commented Aug 21, 2018

If possible, this immediately solves high availability by allowing validator node redundancy without any coordination. Which signature/publickey the network should select can either be a priority field or some form of a sorting order.

This would mess with double-signing prevention. We would still need to punish for signing the same vote with different keys, which means the HA validator still needs to co-ordinate to avoid double signing.

Or maybe you're proposing that only one key is active per height? This way if one key's HSM fails, you can still sign for every other height?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants