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

CIP-0036 | Not allow duplicated vote-key entries #335

Closed
wants to merge 5 commits into from

Conversation

gitmachtl
Copy link
Contributor

Added lines to make it clear that duplicated voting key entries (delegating weighted voting power) are not allowed in the registration metadata format and should be handled as invalid if such entries are included. This keeps away confusion about how the weight for each voting key should be calculated and its also mandatory for hw-wallets showing the right amount of weight for each voting key entry. @janmazak

Added lines to make it clear that duplicated voting key entries (delegating weighted voting power) are not allowed in the registration metadata format and should be handled as invalid if such entries are included. This keeps away confusion about how the weight for each voting key should be calculated and its also mandatory for hw-wallets showing the right amount of weight for each voting key entry.
@rphair rphair added the Update Adds content or significantly reworks an existing proposal label Sep 24, 2022
@rphair rphair changed the title Not allow duplicated vote-key entries (CIP-0036) CIP-0036 | Not allow duplicated vote-key entries Sep 24, 2022
@rphair
Copy link
Collaborator

rphair commented Sep 24, 2022

removed the 'CBOR' from the byte array in the example, because its not a CBOR byte array. if it would be a CBOR byte array we would need to have the CBOR major type 4 in it with the length of the byte array. which is not the case. its simply a byte array of the public_keys/rewards_address.
- added the condition, that the publicStakeKey is not also present in the delegation array of the vote-public-keys. they are derived from different paths, so if there is a match a wrong vote-public-key was used, or the signing stake-secret-key is a wrong one.
@gitmachtl
Copy link
Contributor Author

Also, why is the weight starting at 0? That makes no sense and would only lead to divison/0 errors. There is no reason someone would include a vote public key in the delegation array with weight 0 and other entries with weight>0. So the smallest weight value should be set to 1 imo.

@Scitz0
Copy link
Contributor

Scitz0 commented Sep 28, 2022

Having clear rules defined in CIP for how to deal with edge cases like this is beneficial for wallet implementations.
The two proposed changes to clarify that there should not be duplicate voting key entries and that vote weigt need to be > 0 sounds like a good addition to me.

Copy link
Contributor

@SebastienGllmt SebastienGllmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the Catalyst team signs off

@gitmachtl
Copy link
Contributor Author

So, we had a discussion in the last days on Slack in the iog-governance channel. Outcome is to allow duplicated keys. Signing-/Generation-Tools can still decline to do it that way, but the Catalyst receiving side will allow duplicated key in the delegation array. Also, its ok to show each individual entry of the delegation on the gui of the hw-wallet, and not doing a sum-up. So there is no need to check for duplicated keys during the hw-wallet signing process. Also, vote-weight=0 is a valid value. Because Catalyst with ID=0 is only one usecase, and there might be other voting where 0 could be an important value.

Another thing came up, that there should be an info that using an empty delegation-array equals to a deregistration. So an empty delegation array is also allowed. An additional check i came up with was that none of the vote-public-keys are the same as the public-signing-key. The vote keys are derived from a new path, the signing key is the stake key. So if any vote-public-key is equal to the public key of the signing key -> something is wrong. Either that specific vote-key is wrong, or the signing key is.

With all these changes, i think i have to form a new PR...

@rphair rphair added the State: Likely Deprecated Close if confirmed deprecated (or long waiting). label Oct 1, 2022
@gitmachtl
Copy link
Contributor Author

ok, so when a vote-weight=0 is a valid value in the delegation array, why is than the condition
The weights in the delegation array are not all zero in CIP-0036 a needed condition?

We allow to set the vote-weight to zero, but not for all vote-public-keys? An empty delegation array is now seen as a deregistration, wouldn't be a delegation array with all vote-weights set to zero also just result in the same outcome?

@gitmachtl
Copy link
Contributor Author

gitmachtl commented Oct 1, 2022

This is now a valid registration, which would remove all delegated voting power:

{
  "61284": {
    "1": [],
    "2": "0x57758911253f6b31df2a87c10eb08a2c9b8450768cb8dd0d378d93f7c2e220f0",
    "3": "0xe0c13582aec9a44fcc6d984be003c5058c660e1d2ff1370fd8b49ba73f",
    "4": 1234567,
    "5": 0
  },
  "61285": {
    "1": "0xb8382668e6e65c42718412f297a3c35d3beabbf85d57560f9c9658e1e2edb40a3f036b07afffa31745242f9b02cf89a59e9ba6391e83da2fe7b8c484b913640c"
  }
}

And this would also be valid if we allow vote-weight=0 and duplicated entries, but not according to the current CIP rules:

{
  "61284": {
    "1": [
      [ "0x51f117d26e29aea7db3d1f2f874ab5f585f619a95aed6d71d31a7404cb6557b5", 0 ],
      [ "0x51f117d26e29aea7db3d1f2f874ab5f585f619a95aed6d71d31a7404cb6557b5", 0 ]
    ],
    "2": "0x57758911253f6b31df2a87c10eb08a2c9b8450768cb8dd0d378d93f7c2e220f0",
    "3": "0xe0c13582aec9a44fcc6d984be003c5058c660e1d2ff1370fd8b49ba73f",
    "4": 1234567,
    "5": 0
  },
  "61285": {
    "1": "0xdcd2a450f4a9a6790a4e0147b26a7be3b75d297d18a99405be6f15b07bf8f45df98d26a2ecd9d4d0a118d9f003ffd6226bc5fcf2fe48c68a8e045d2439ede600"
  }
}

Its a bit inconsistent. On one hand we're not saying that the vote-weight should be > 0 to avoid "invalid" delegations, on the other hand its not allowed to have vote-weight=0 for all vote-public-keys? Having a total vote-weight of zero should also result in removing all the delegated voting power like a "deregistration". In that way it would be consistent.

@gitmachtl
Copy link
Contributor Author

also related to allow exporting vote-public-keys on the wallets with a known prefix:
#342

@gitmachtl
Copy link
Contributor Author

closed, will be done via a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Likely Deprecated Close if confirmed deprecated (or long waiting). Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants