-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Comments
Pretty sure this is functioning as expected, I'm assuming that this key Also note that each validator must have a delegation to itself- this is referred to as the self-delegation (or sometimes I'm going to close this issue but feel free to still ask questions if my above explanation doesn't clear things up |
@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 |
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. |
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. |
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 by signing key you mean Tendermint signing key - then yes
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.
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
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.
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 |
@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. |
agreed |
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? |
Summary of Bug
In the
genesis.json
file thebonds
list generated bygaiad init
or filled out by testnet participants contains two addresses, one for the delegator's address and another for the validator's address.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 becosmosvaladdr17zxzlwh0842czgxk7hcj60enwx9368kg452u56
.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
genesis.json
file.For Admin Use
The text was updated successfully, but these errors were encountered: