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-36? | Add support for Catalyst multi-delegation #200

Closed
wants to merge 5 commits into from

Conversation

mark-stopka
Copy link
Contributor

Create repacement to CIP-15 to support the latest requirements for Catalyst and Voltaire, namely the ability to delegate one's rights to vote to (possibly multiple) representatives and/or expert voters.

@mark-stopka
Copy link
Contributor Author

@SebastienGllmt, @KtorZ what changes should be made, so this can me merged on the next CIP review meeting?

@KtorZ
Copy link
Member

KtorZ commented Feb 2, 2022

@mark-stopka it won't be merged in the next CIP meeting; but it may go into last-check. The process is lengthy on purpose to allow folks to comment and react on proposals. As for the content itself, I haven't looked at it in depth yet but the overall looks good and well-structured. So I don't foresee any particular issue given that we already discussed the approach in the last meeting.

Copy link
Contributor

@disassembler disassembler left a comment

Choose a reason for hiding this comment

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

Is this really what we want for multi-delegation? This allows you to split your voting rights across multiple keys with different weights, which might be useful, but I always assumed multi-delegation for voting was going to be the ability to designate different experts for different challenges with the full voting power... e.g. I might delegate rights to the SPO I delegate to for an SPO challenge.

Notably, there should be five entries inside the metadata map:
- A non-empty array of delegations, as described below;
- A stake address for the network that this transaction is submitted to (to point to the Ada that is being delegated);
- A Shelley address discriminated for the same network this transaction is submitted to to receive rewards.
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to be a shelley stake address. It can't be a regular address because of minutxo requirements.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that the challenges are not static -- they dynamically change every round. It means you can't "fire and forget" for a specific category and you'd have to explicitly re-delegate your vote every funding round. You could argue whether or not this is a desirable property or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would argue it is a desirable property for now; later after DCF emerges and voting on parameters will be enabled we will have to revisit this CIP anyway; so IMO for now it fits the current crucial need to enable Project Catalyst to grow and get us where we need to be this year.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the voting purpose field allows different delegation assignments for specific reasons (and the CIP deliberately doesn't restrict what those reasons might be). So we simply need to define the mappings from voting purpose to challenges etc. and reflect that in the voting weights.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can explicitly make the rewards address a Shelley address. I think that was just carried over from CIP-15

Copy link
Contributor

Choose a reason for hiding this comment

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

Mark, we are definitely looking ahead to non-Catalyst uses for this

@MichalPetro
Copy link

MichalPetro commented Feb 4, 2022

We discussed this today and there are two options how to implement this proposal in hw wallets:

  1. Drop the old format and support only the proposed one. We would need to sync the breaking change with sw wallets.
  2. Support both formats, even on fw level. This would increase fw code complexity and size (significantly).

We prefer the first option since it is easier to implement and doesn't increase hw wallet app size that much.

@kevinhammond
Copy link
Contributor

We need to add the following text to this CIP. I can't do that.

Voting key derivation path

To avoid linking voting keys directly with Cardano spending keys, the voting key derivation path must start with a specific segment:

m / 1694' / 1815' / account' / chain / address_index

@crptmppt
Copy link
Contributor

crptmppt commented Mar 15, 2022

discussed in Meeting 41 - requesting derivation path to be added /approved by @mark-stopka
& add original author for credit (Giacomo)

@KtorZ KtorZ changed the title Add support for Catalyst multi-delegation CIP-36? | Add support for Catalyst multi-delegation Mar 17, 2022
- A stake address for the network that this transaction is submitted to (to point to the Ada that is being delegated);
- A Shelley address discriminated for the same network this transaction is submitted to to receive rewards.
- A nonce that identifies that most recent delegation
- A number that indicates the purpose of the vote
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be more specific here, is the "number" a non-negative integer?

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the CDDL below, the purpose number is optional. Shouldn't it be mentioned here? Or some discussion of allowed/typical/default values of that?


### Delegation format

A delegation assigns (a portion of) the ADA controlled by one or more UTxOs on mainnet to the voting key in the sidechain as voting power. The UTxOs can be identified via the stake address at some designated point in time.
Copy link
Contributor

Choose a reason for hiding this comment

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

I can only guess what at some designated point in time means here.

- a voting key: simply an ED25519 public key. This is the spending credential in the sidechain that will receive voting power from this delegation. For direct voting it's necessary to have the corresponding private key to cast votes in the sidechain. How this key is created is up to the wallet.
- the weight that is associated with this key: this is an unsigned integer (CBOR major type 0) that represents the relative weight of this delegation over the total weight of all delegations in the same registration transaction.
The weight may range from 0 to 2^32-1. Any greater value is capped to 2^32-1.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should specify the maximum number of delegations per transaction. (I wouldn't go above 2^16-1.)

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no technical need to fix a maximum number of delegations - it will be restricted by the transaction size. The practical limit will be around 200-300

4. Any remaining voting power is assigned to the last voting key in the delegation array.

This ensures that the voter's total voting power is never accidentally reduced through poor choices of weights,
and that all voting powers are exact ADA.
Copy link
Contributor

Choose a reason for hiding this comment

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

"voting powers are exact ADA" sounds sloppy

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll revise this. It came from the original CIP-15, I believe


Voting registration example:
```
61284: {
Copy link
Contributor

Choose a reason for hiding this comment

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

61284 should be explicitly mentioned before it is used in an example.

Copy link
Contributor

Choose a reason for hiding this comment

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

CIP-0036/schema.cddl should also be referenced before the example.

The advised way to construct a nonce is to use the current slot number.
This is a simple way to keep the nonce increasing without having to access
the previous transaction data.
- The reward address is a Shelley address discriminated for the same network
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned by @disassembler , it should be a stake address.

This data is signed with the staking key as follows: first, the
blake2b-256 hash of `sign_data` is obtained. This hash is then signed
using the Ed25519 signature algorithm. The signature metadata entry is
added to the transaction under key 61285 as a CBOR map with a single entry
Copy link
Contributor

Choose a reason for hiding this comment

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

"added to the transaction" is imprecise, we are still talking about constructing tx metadata, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, would "The signature metadata is added to the transaction metadata under key 61285" be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, definitely.

'2': '0x1c5d88aa573da97e5a4667e0f7c4a9c6a3d848934c3b0a5b9296b401540f2aef',
'3': '0xe0ae3a0a7aeda4aea522e74e4fe36759fca80789a613a58a4364f6ecef',
'4': 1234,
'5': 0
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure one can call this "legacy" --- it does not comply with CIP-15 which does not contain key 5.

Copy link
Contributor

Choose a reason for hiding this comment

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

correct, this (and the related occurrence above) should be changed

61284: {
// delegations - CBOR byte array
1: [("0xa6a3c0447aeb9cc54cf6422ba32b294e5e1c3ef6d782f2acff4a70694c4d1663", 1), ("0x00588e8e1d18cba576a4d35758069fe94e53f638b6faf7c07b8abd2bc5c5cdee", 3)],
// stake_pub - CBOR byte array
Copy link
Contributor

@SebastienGllmt SebastienGllmt Mar 30, 2022

Choose a reason for hiding this comment

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

Is there a reason only allow public keys and not script hashes? Only supporting public keys would block multisigs from delegating their vote. It wouldn't add any complexity to the voting app because at the end of the day you're delegating to a (non-script) voting key

Probably the type should be
vkey | native_script since they both can be easily checked, but probably handling of Plutus multisig wallets is not doable in time for fund 9

Copy link
Contributor

Choose a reason for hiding this comment

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

No particular reasons besides increased complexity on the backend to validate such scripts. Plutus for sure seems out of scope for now, but I'm worried even native scripts might be a bit of a stretch atm, considering required changes to the format as well.
Do we know how common / requested this feature is? We can add such functionality in the future if needed

Copy link
Contributor

Choose a reason for hiding this comment

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

@zeegomo I think right now there's 100M+ locked in native script multisigs including ADA owned by the Milkomeda DAO

@KtorZ
Copy link
Member

KtorZ commented Apr 5, 2022

@kevinhammond @mark-stopka @zeegomo

In the absence of Mark, I have updated the original PR from Giacomo to include commits from this PR that made the CIP a standalone one (I also took the liberty to amend the authors accordingly, removing Sebastien, Rhinor and Mikhail from the list, but adding Mark to it).

As discussed during today's CIP meeting, we agreed that in order to proceed with this proposal, the CDDL format for the voting key must be changed to be forward-compatible with the future introduction of different types of credentials (e.g. native multi-sig scripts) and not only staking keys. This could be achieved by representing it as a classic sum-type using tags in CBOR.

The changes will be done on #188.

@KtorZ KtorZ closed this Apr 5, 2022
@gitmachtl
Copy link
Contributor

gitmachtl commented Sep 19, 2022

@kevinhammond @mark-stopka @zeegomo

What would be the outcome if you use a vote public key multiple times in the registration/delegation like:

{
61284: {
   1: [
      [h'51F117D26E29AEA7DB3D1F2F874AB5F585F619A95AED6D71D31A7404CB6557B5', 1],
      [h'57758911253F6B31DF2A87C10EB08A2C9B8450768CB8DD0D378D93F7C2E220F0', 1], 
      [h'51F117D26E29AEA7DB3D1F2F874AB5F585F619A95AED6D71D31A7404CB6557B5', 2], 
      [h'51F117D26E29AEA7DB3D1F2F874AB5F585F619A95AED6D71D31A7404CB6557B5', 17] 
      ],
   2: h'51F117D26E29AEA7DB3D1F2F874AB5F585F619A95AED6D71D31A7404CB6557B5', 
   3: h'E0C13582AEC9A44FCC6D984BE003C5058C660E1D2FF1370FD8B49BA73F', 
   4: 123456789, 
   5: 0
   }
}

Will it only count a single time, or will the voting power accumulate? Or is this an invalid delegation/registration at all?

@gitmachtl
Copy link
Contributor

I would propose, to add a line in the CIP that is saying: "no duplicated public-voting-key entries are allowed, otherwise such a delegation metadata content would be invalid".

Frontend tools can also take care to make it clear for the user.

@gitmachtl
Copy link
Contributor

Created a PR: #335

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

Successfully merging this pull request may close these issues.

10 participants