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-0126? | Multi-Stake Delegation from a Single Account #628

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

AngelCastilloB
Copy link

@AngelCastilloB AngelCastilloB commented Nov 30, 2023

CIP for multi-stake delegation from a single account.

Human-readable version.

@AngelCastilloB AngelCastilloB force-pushed the multistake-delegation-from-single-account branch from 8ee8d07 to 2744bb2 Compare November 30, 2023 08:06
@Ryun1 Ryun1 changed the title Initial draft CIP-???? | Multi-Stake Delegation from a Single Account Nov 30, 2023
@Ryun1 Ryun1 added the Category: Wallets Proposals belonging to the 'Wallets' category. label Nov 30, 2023
@yHSJ
Copy link

yHSJ commented Nov 30, 2023

Thanks for submitting this! I left the following feedback in the Lace Discord, but felt I should share it here too:

My initial feedback would be that you should use RFC-2119 to strictly define the language use, and then modify the specification to use that language. This is feedback I've given to almost every CIP. Cardano's standards right now use a lot of ambiguous language that one could argue is implicitly defined, but as an implementer I've seen several different interpretations of standards. If RFC-2119 was used, that ambiguity is removed.

CIP-XXXX/README.MD Outdated Show resolved Hide resolved
CIP-XXXX/README.MD Outdated Show resolved Hide resolved
Therefore, the following key principle will guide the balancing change process for wallets that are multi-delegating:

Every multi delegation transaction (creation or update) must include the delegation portfolio metadata specifying the percentage of funds they wish to delegate to each pool. This portfolio serves as a guideline for how funds should be distributed across different stake keys.
When following transactions are built, input selection is performed as usual. The resulting change from the transaction is then evaluated for rebalancing. The structure and size of change outputs determined by the input selection algorithm are preserved.

Choose a reason for hiding this comment

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

When initially starting multi-delegation, Lace seems to try to ensure that UTxOs of all sizes are equally available on all of the stakes. Might be worth mentioning here?

Copy link

Choose a reason for hiding this comment

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

I think this is an implementation detail. Wallets may choose to leave the distribution of UTxOs up to the user, for example.


Every multi delegation transaction (creation or update) must include the delegation portfolio metadata specifying the percentage of funds they wish to delegate to each pool. This portfolio serves as a guideline for how funds should be distributed across different stake keys.
When following transactions are built, input selection is performed as usual. The resulting change from the transaction is then evaluated for rebalancing. The structure and size of change outputs determined by the input selection algorithm are preserved.
The change outputs will be distributed as-is across multiple addresses associated with different stake keys, without altering the outputs values. Priority is given to stake keys that are most divergent from the set preferences, thereby gradually realigning the actual distribution with the user's delegation strategy.

Choose a reason for hiding this comment

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

Why not changing the output values? If it is a transaction created in the soverignty of the wallet app, it can choose the output sizes freely and can probably better redistribute if it does.

Nit-pick: If the stake is divergent in the direction of too much stake, priority should specifically not be given to this stake. Could maybe be worded a bit clearer.

Copy link
Author

@AngelCastilloB AngelCastilloB Dec 1, 2023

Choose a reason for hiding this comment

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

Early we made the design choice to not require changes on the input selection. These change outputs are produced by selection algorithms that are design to prevent dust and in theory optimize the UTxO set of the wallet. If the wallet is already using any given strategy for UTxO selection (and change production), implementing this CIP shouldn't force the wallet to alter that if possible. In our tests, only redistributing the change outputs produced as they are was enough to subvert dirft given enough time


The inclusion of the delegation portfolio object as transaction metadata serves to reflect any updates to the user's delegation preferences on-chain. This ensures that external applications retrieving this information are always presented with the most current data, without the need to parse through multiple transactions.

External applications can retrieve the most up-to-date delegation portfolio for a given wallet by scanning the blockchain for the latest transaction with the metadata label 6862 originating from the user's wallet. This transaction will contain the metadata with the current delegation portfolio, thereby affirming the wallet owner's latest delegation preferences.

Choose a reason for hiding this comment

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

Scanning historical transactions for a specific metadatum label is rather inefficient. For wallet apps, that's a minor problem since they will typically want to present the whole transaction history to the user anyway and they only deal with one or a small set of wallets. But for dApps that are confronted with ever new wallets every moment, it might well be prohibitive.

Copy link
Author

Choose a reason for hiding this comment

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

Agree here, but what could be the alternative? Also for external applications that need this information, they could build any off-chain solution to make this efficient.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To be honest, if this standard were to be adopted the only way I would support it as a dApp developer is if the wallet itself either:

  1. Figured out its changing strategy for me
  2. At least provided the connected wallet's changing strategy

- Receiving funds to a single address
- Building transactions with software that doesn’t support multi-stake delegation (e.g. via dapp connector)

In order to maintain a record of user’s delegation portfolio preferences , all actions pertaining to stake delegation, including funds transfers, stake key registration, and deregistration, must be executed within a single atomic transaction when performing an update. This transaction should also include the delegation portfolio metadata, using the reserved metadata label 6862 (hexadecimal 0x1ace), as outlined in [CIP-0017](https://github.com/cardano-foundation/CIPs/tree/master/CIP-0017) preferably without any of the optional fields.

Choose a reason for hiding this comment

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

Maybe make even more explicit that wallet apps must ensure that the chosen pools match between the CIP-17 JSON specification and the pools actually delegated to. … and that other wallet apps managing the same account are free to correct it if one misbehaves.

Should a new delegation certificate be included for all stakes and pools even if there is no change in that part of the multi-delegation? Advantage would be that this ensures that all stake keys have to sign the transaction partly attenuating @yHSJ's point on “Franken” addresses above. Disadvantage would be that it bloats the transaction and makes it more expensive unnecessarily.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make even more explicit that wallet apps must ensure that the chosen pools match between the CIP-17 JSON specification and the pools actually delegated to. … and that other wallet apps managing the same account are free to correct it if one misbehaves.

I like this idea

… partly attenuating …

Since it doesn’t fully attenuate I’d vote in favour of not enforcing unnecessary certificates.

#628 (comment)


Balancing change is a crucial feature designed to maintain a user’s stake distribution, using the portfolio preferences attached to the delegation transaction. As transactions occur over time, the distribution of funds across different pools will deviate from the initial portfolio percentages as the change would otherwise be concentrated onto a single address, and therefore be delegated to a single pool. This drift can be corrected while constructing transactions for a user who has defined a multi-delegation intent by reading the preferences from the delegation transaction metadata and distributing the change to best adhere

When implementing this feature, it is essential to maintain the integrity of the change outputs as produced by the input selection algorithm. The design of these algorithms, such as the Random Improve input selection, is aimed at optimizing the overall health of the wallet's UTXO (Unspent Transaction Output) set. This optimization includes avoiding the creation of dust and ensuring that the size of outputs aligns with the user's spending habits over time.

Choose a reason for hiding this comment

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

Is Random Improve really a good coin selection algorithm for this? Is it – in an unmodified form – even a good coin selection algorithm for wallets with native tokens?

Shouldn't it at least be modified to select UTxOs from all stakes as proportional as possible and not leave that to the randomness of the algorithm?

Copy link
Author

Choose a reason for hiding this comment

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

Random Improve was just provided as an example. The point here would be that implementing multi-delegation shouldn't require changing the current input selection algorithm to accommodate it, the wallet could be using an input selection strategy that optimize for a given use case, ideally we dont want to disrupt that, as forcing the wallet to come up with a new input selection algorithm that satisfies his use case and multi delegation could be too problematic. Also during our internal tests, just directing the change output was sufficient to advert drift in the long run


Every multi delegation transaction (creation or update) must include the delegation portfolio metadata specifying the percentage of funds they wish to delegate to each pool. This portfolio serves as a guideline for how funds should be distributed across different stake keys.
When following transactions are built, input selection is performed as usual. The resulting change from the transaction is then evaluated for rebalancing. The structure and size of change outputs determined by the input selection algorithm are preserved.
The change outputs will be distributed as-is across multiple addresses associated with different stake keys, without altering the outputs values. Priority is given to stake keys that are most divergent from the set preferences, thereby gradually realigning the actual distribution with the user's delegation strategy.

Choose a reason for hiding this comment

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

The problem of NFTs and other native tokens should at leas be mentioned.

Is it maybe feasible to demand that native tokens are not moved between stakes in the process that they stay on the same stake address unless the user explicitly moves them to another stake?

Copy link
Author

@AngelCastilloB AngelCastilloB Dec 1, 2023

Choose a reason for hiding this comment

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

could you elaborate more on the problem of NFTs and other native tokens?

Adding this extra requirement of allowing users to fix tokens to specific stake address would make the input selection harder, and many edge cases would need to be considered.

In short this will requiere wallets to modify their input selection algorithm. We want to avoid having as a requirement to support this having to alter the input selection algorithm, doing this properly is not trivial, some wallets may be relying on libraries that do the input selection and could not have control over this. So having as a requirement modifying/coming up with a new input selection algorithm being use by wallets seems too harsh. The approach followed by this CIP (simply redirecting change outputs) is compatible with any input selection, is easy to implement and ultimately achieve the end goal of avoiding drift

@AndrewWestberg
Copy link
Contributor

This is basically the approach Lace wallet takes. I disagree entirely with this approach. It has the following negative consequences:

1.) Requires registering of additional stake addresses.
2.) The rewards claiming transaction becomes more complex.
3.) dApps that also build transactions outside the wallet know nothing about this approach and won't care about re-balancing.

A better approach is to simply change the delegation certificate transaction to allow for multi-pool delegation. This way, you only need a single stake key. Wallets do not need to change much. dApps do not need to change at all.

The necessary changes are then constrained to updates to the ledger to track multi-delegation rewards properly. No re-balancing of utxos is necessary. Everything moves to just being math inside the ledger staking/rewards system.

@AndrewWestberg
Copy link
Contributor

image
Just do it here. Change it to an array where you can specify poolid/percentage. Then make the necessary changes on the ledger team behind a hardfork flag. All math and updates are nicely self-contained in the node instead of bleeding out to community wallets, dapps, etc...

{
"pools": [
{
"ticker": "DARK",
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that putting stake keys here instead of pool ids/tickers would be better, because:

  • there wouldn't be a need to update the metadata if delegation changes, but not the weight
  • wallet implementations will have to perform one series of lookups less: instead of caring about pools, they will be only concerned with stake keys
  • removing a delegation in favor of another would be as easy as re-delegating stake for one stake key - again, no metadata update

Copy link
Contributor

Choose a reason for hiding this comment

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

As a separate note, there is probably no need in supporting tickers at all: reverse lookups only introduce complexity for the implementors to deal with

Copy link
Collaborator

@rphair rphair Dec 1, 2023

Choose a reason for hiding this comment

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

latter #628 (comment) appears resolved here: #628 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

@rphair no it does not: that discussion was about tickers vs pool IDs, but I propose using stake addresses instead of pool identifiers.

Copy link
Contributor

@klntsky klntsky Dec 4, 2023

Choose a reason for hiding this comment

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

dApps that will actually make use of this standard may not care about delegations at all. So why force them to perform additional lookups to figure out which of the stake addresses correspond to which pools?

Copy link
Collaborator

@rphair rphair Dec 5, 2023

Choose a reason for hiding this comment

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

thanks @klntsky I was only referring to your 2nd #628 (comment) in my own #628 (comment); apologies for the ambiguity & I've corrected that now. BTW I would support the suggestion to use stake addresses vs. pool IDs.

Copy link
Author

@AngelCastilloB AngelCastilloB Dec 5, 2023

Choose a reason for hiding this comment

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

Regarding changing the pool ids for stake keys, this is an interesting idea, but I am not convinced yet this is better. First, wallets need this mapping anyways since in order for wallet software to determine to which pools the users is delegating to, it needs to first find the stake keys of the user and then find to which pools the keys are delegated to. The users also interact with the delegation from the point of view of pools not stake keys, as in, users select pools and percentages and this information would need to be converted to key hashes in order to create the metadata as you are suggesting. This also have the opposite effect on external actors, anyone that wants to parse this metadata to determine the user portfolio will now have to do an extra lookup to find which pools these stake keys are delegating to, as it is right now no extra lookup are required for external actors, and the wallet software already can (and need to anyway in both cases) map from stake key to pool id.

there wouldn't be a need to update the metadata if delegation changes, but not the weight

There wouldn't be the need for updating the metadata in the case where pools are swapped but no number of pools or percentages changes, agree.

wallet implementations will have to perform one series of lookup less: instead of caring about pools, they will be only concerned with stake keys

The lookup cant be avoided, users delegate to pools and this would need to be converted to stake keys anyways to issue certificates. The mapping Stake Key -> Pool cant be avoided by the wallet.

removing a delegation in favor of another would be as easy as re-delegating stake for one stake key - again, no metadata update

This is the same as point 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

The users also interact with the delegation from the point of view of pools not stake keys, as in, users select pools and percentages and this information would need to be converted to key hashes in order to create the metadata as you are suggesting

You are only considering wallet's and user's point of view. But what about dApps that just want to generate change such that the distribution is preserved. These dApps may as well be other wallets that do not fully support this feature (i.e. in the UI), but try to respect the distribution.

I think it's obvious that one can always go from poolIDs to stake addresses and in the opposite direction. Stake addresses just have strictly more use cases without an extra lookup

@JaredCorduan
Copy link
Contributor

@AndrewWestberg 's suggestion is indeed much cleaner, and indeed this has been suggested many times over the years.

TLDR - I think many people that think it is a good idea, it will just take time. So it's a matter of how soon do people want progress on multi-delegation.

The reasons the ledger change was never implemented were:

  • the reward calculation is already a beast, and much effort was put into minimizing the consequences of this massive calculation. nobody wanted to make the situation worse until after the "big data structures" were moved out of memory. once this is done, the calculation can be done in a separate process and the worry is gone. (yes, it could be moved to a separate processes before it was moved out of memory, but that then just exacerbates the already big task of moving stuff out of memory...).
  • the reward calculation is not currently easily swappable. What looks like a very easy change to the CDDL, and an easy change to the reward calculation, requires more refactoring than is immediately obvious.

@Scitz0
Copy link
Contributor

Scitz0 commented Dec 1, 2023

I recognize the issues raised by Jared but I fully agree with Andrews suggestion to try and raise the priority to get this included in the ledger instead of making various patch-work on top that will create a ton of bad UX, constant rebalancing of wallets, and a bloated utxo set. We have lived without "proper" multi-delegation for over three years, and I don't see a huge need for it. Account delegation has served us sufficiently so far I would say. So if to be implemented, let's do it properly.

@gitmachtl
Copy link
Contributor

image Just do it here. Change it to an array where you can specify poolid/percentage. Then make the necessary changes on the ledger team behind a hardfork flag. All math and updates are nicely self-contained in the node instead of bleeding out to community wallets, dapps, etc...

I can double down on that. Since the beginning with Shelley we had an array output for a query stake-address-info on the CLI. And thats for a reason, because the idea was to list multiple reward-sources there. So yes, the best solution would be to talk to the ledger team and figure out a way to have a delegation/weight array instead of a single pool_keyhash in there.

@gitmachtl
Copy link
Contributor

TLDR - I think many people that think it is a good idea, it will just take time. So it's a matter of how soon do people want progress on multi-delegation.

I think people are now familiar with "delays" on cardano. Creating a difficult workaround where you can ruin everything with a single wrong transaction is a bad replacement imo. Wallets can already automatically split an account into multiple accounts if people want to. I would like to see a ledger based solution here - even if it takes longer - than a complicated workaround. Oh by the way @JaredCorduan - Its lovely to hear from you. :-)

@yHSJ
Copy link

yHSJ commented Dec 1, 2023

This is basically the approach Lace wallet takes. I disagree entirely with this approach.

I would agree this feels like an incomplete solution to a problem that can, and should, be solved at the ledger level for a number of reasons. That being said, I also think it's worth considering how a wallet could choose to implement multi-stake key accounts and I applaud the Lace team for working within the bounds that the ledger has defined currently.

It is also worth mentioning due to the reaction that I saw on twitter that a CIP is just that -- a proposal. Even if it were to get merged it does not become network "law". It would sit as Proposed until it's completed it's "Path to Active". In general, it's really great to see so many people engaging with the process and sharing their thoughts on multi-delegation!

@QuasarChains
Copy link

I would like to ask that someone here look into the process Gamechanger Wallet uses for multi-delegation both for single account and also additional accounts and key derivations. CIP-30 does not support key derivations. Similar to other wallets and the ability to have different accounts, GameChanger calls these workspaces. Here's a link to the repo with an outline of the code and process for multi-delegation.

I think this is worth reviewing because I think an answer that is best for Cardano is closer than it may appear and one that allows anonymity a chance to remain prevalent.

Gamechanger is a web wallet so there is no download and the testnet version comes with an airdrop into a burner wallet so there is no password and no mnemonics and there's a built-in IDE to write/review/edit the code. Link to testnet wallet for easy access. Gamechanger is also a DApp so you can connect the Lace wallet along with any other browser extension to get a better view on how to build things so that they are composable and interoperable within the ecosystem.

Gamechanger and Lace handle multi-delegation differently and that is why it deserves this group's attention.
Thank you.

@rphair
Copy link
Collaborator

rphair commented Dec 2, 2023

@zxpectre would you be willing to bring your perspective into this discussion as suggested by @QuasarChains in #628 (comment)?

@ccgarant
Copy link
Contributor

ccgarant commented Dec 2, 2023

I'm in favor of @AndrewWestberg's recommendation because it's the simpler top level change and fundamentally correct.

I don't think there's a huge rush that we can't do the correct change in the ledger. Per @JaredCorduan's comments it sounds like no small feat, but it should definitely be in the roadmap imho.

Cardano's self-custody staking is a shining light, phase 2 should have multi-delegation done properly in the ledger.

A better approach is to simply change the delegation certificate transaction to allow for multi-pool delegation. This way, you only need a single stake key. Wallets do not need to change much. dApps do not need to change at all.

Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

Hey all, thanks for the proposal.

I have left a few editorial comments on the structure throughout.

A larger question, that I don't see discussion of - is the potential impact of DRep delegations?

  • Should a multi-delegation scheme also cater for DReps?
  • How should delegations to DReps and Pools be managed?
    • For me this is not straight forward, as due to the design of both mechanisms using mappings from one stake credential.
    • This means that the weightings for Pool delegations must be shared with DRep delegations and vice-versa.
  • What if yet another type of delegation is introduced in the future, how should that work?

CIP-XXXX/README.MD Outdated Show resolved Hide resolved
CIP-XXXX/README.MD Outdated Show resolved Hide resolved
CIP-XXXX/README.MD Outdated Show resolved Hide resolved
CIP-XXXX/README.MD Outdated Show resolved Hide resolved
By linking all stake keys to the first payment key, this proposal introduces a transparent and simple mechanism for multi-delegation, greatly simplifying the fund tracking process. This association not only simplifies the delegation and deregistration processes but also facilitates easier tracking of funds and delegations. Besides, this method does not preclude the use of the multi-account approach for users desiring enhanced privacy. Stakeholders can still derive multiple accounts, each potentially capable of multi-delegation, granting them granular control while retaining the option for increased privacy.

### Foundation for Advanced Wallet Features and Enhanced User Experience
One of the underlying rationales of this proposal is to provide a robust foundation for more advanced wallet features. The dynamic nature of this approach offers wallets the opportunity to innovate. For example allowing users to support multiple dReps from a single account, or implementing sophisticated heuristics to maintain stake distribution to different pools. .
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
One of the underlying rationales of this proposal is to provide a robust foundation for more advanced wallet features. The dynamic nature of this approach offers wallets the opportunity to innovate. For example allowing users to support multiple dReps from a single account, or implementing sophisticated heuristics to maintain stake distribution to different pools. .
One of the underlying rationales of this proposal is to provide a robust foundation for more advanced wallet features. The dynamic nature of this approach offers wallets the opportunity to innovate. For example allowing users to support multiple DReps from a single account, or implementing sophisticated heuristics to maintain stake distribution to different pools. .

Capitalization.

But also, how could this be expanded for DRep delegation? this is not clear to me.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the feedback @Ryun1, as you mentioned in your comment, DRep vote delegation and Stake delegation use the same key, so the two systems are intertwined at the protocol level, the only important consideration this CIP adds regarding DReps is that users will use more than one stake key and can have funds distributes among those stake keys, this will mean that users must also delegate their vote to the DReps for each key, for example:

Alice have a portfolio delegating to three pools, A, B and C with the following distribution 10%, 50%, 40%. She can chose to either delegate all three stake keys to the same DRep (effectively delegating 100% of her voting power to him) or delegate her different stake keys to different DReps, for example if she want to distribute her voting power between two DReps, she could delegate the same stake key used for pool A and B to DRep1 and the remaining stake key to DRep2 delegating to each each (approximately) 50% of her voting power, alternatively she could delegate to three different DReps. This is how the system works at the protocol level, the difference here is that wallets that implement this CIP will have the additionally flexibility to allow Alice to delegate to more than one DRep if she has more than one stake key.

As for how this would work with future types of delegations is hard to say since it will depend on the mechanics of those, but this CIP is not proposing/altering any feature at the protocol level, having multiple stake keys is already supported by the protocol, so I would assume there will be no conflict with any future use of stake keys unless there is a breaking protocol change in that regard.


The inclusion of the delegation portfolio object as transaction metadata serves to reflect any updates to the user's delegation preferences on-chain. This ensures that external applications retrieving this information are always presented with the most current data, without the need to parse through multiple transactions.

External applications can retrieve the most up-to-date delegation portfolio for a given wallet by scanning the blockchain for the latest transaction with the metadata label 6862 originating from the user's wallet. This transaction will contain the metadata with the current delegation portfolio, thereby affirming the wallet owner's latest delegation preferences.
Copy link
Contributor

Choose a reason for hiding this comment

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

for the latest transaction with the metadata label 6862 originating from the user's wallet

There's no such thing as user's wallet. There are utxos at addresses that are being consumed. Consider this: I construct an atomic swap transaction and put metadata that redefines the portfolio. I send it to the swap counterparty and they sign it. Who of us now has the delegation preferences updated?

Or even simpler, I am a malicious NFT minting dApp that wants to steal user's delegation. I put a metadata hash but do not disclose it to the user (which is normally expected)...

It is important to note that currently metadata posting never imposes any financial consequences for the users. But with this proposal adopted, it will.

This should not be accepted without a clear algorithm how to go from a chain of transactions to a mapping of payment_address -> delegation_map

Copy link
Author

@AngelCastilloB AngelCastilloB Dec 5, 2023

Choose a reason for hiding this comment

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

Thanks for the feedback @klntsky, only updating the metadata doesn't affect the users delegation, the delegation can only be altered by submitting certificates and the right signatures in the transaction, so a malicious dApp cant steal user delegation by only updating the metadata of the transaction. The portfolio metadata is only intended to be consumed by off chain clients for context, I.E to determined the users preferences vs theirs actual stake distribution

Copy link
Contributor

Choose a reason for hiding this comment

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

@AngelCastilloB

delegation can only be altered by submitting certificates and the right signatures in the transaction

They do not specify the amount, so that any ADA that is on a stake pubkey is automatically delegated.

Which means a wallet or a dApp that consumes this info will find the wrongly updated metadata and prompt the user to either perform a rebalance, or will silently generate a change distribution such that ADA is moved to a SP the user didn't want to prefer more.

Right now as I understand there is no well-defined connection between user's "identity" and metadata, and what is this "identity" is also undecided.

Copy link
Author

Choose a reason for hiding this comment

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

Ah I see your point now @klntsky, so yes it would be possible for a malicious dApp to move the percentages, but it wouldn't be possible to make the user delegate to a different pool.

So I guess the attack vector you are suggesting here would be for an adversarial pool to create a dApp that when it detects an user that is multi-delegating and one of the pools is multi-delegating to is in his control, it would then change the percentages to make it more favorable for him.

I think we have a few options here:

  • Add a signature to the metadata
  • Place the metadata in an script output the user controls

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a signature to the metadata

Yep. That would also make it unambiguous who is the issuer of the metadata in case of multiple user inputs. Moreover, to support this edge case, user address or pubkeyhash can be used as a key in the map.

Place the metadata in an script output the user controls

There is no way to prevent consuming an UTxO from another wallet

@Ryun1
Copy link
Collaborator

Ryun1 commented Dec 10, 2023

Hey @AngelCastilloB!

I have put this proposal on the Triage agenda for CIP Editors Call # 78 (December 12th 4pm UTC), held in The CIP Editors Meetings Discord, see agenda and discord event.

Would be great to see you there to discuss this proposal.

@kevinhammond
Copy link
Contributor

This is basically the approach Lace wallet takes. I disagree entirely with this approach. It has the following negative consequences:

... 2.) The rewards claiming transaction becomes more complex; ...

I'm not sure I understand this, don't you simply register the same rewards address for multiple staked addresses? Then the rewards withdrawal transaction is just the same. Lace may not do that, but that's really a UI/UX issue isn't it?

@yHSJ
Copy link

yHSJ commented Dec 12, 2023

There has been a lot of feedback here, on twitter, and in the CIP Discord/meeting specifically regarding difficulties this could introduce for dApps who use the stake key at the first index of the response from CIP-30's getRewardAddresses function as an account abstraction for the dApp (as well as querying the chain, and signing transactions). Some folks have suggested this is a reason to not merge this CIP as that may imply "officiality" and pressure wallets to implement it (further exasperating concerns for dApps).

My response, which I felt should also be shared here, has been that there is already a CIP (CIP-18) that was merged regarding deriving multiple stake keys from a single account. Therefore, this should not be blocked for that reasoning, as it would exist in the same state as CIP-18.

I think it is reasonable, and perhaps ideal, to add some information to the rationale section that discusses this concern, and provides context regarding potential dApp integration problems. Then each dApp and wallet can make their own informed decisions when considering implementation.

@leo42
Copy link
Contributor

leo42 commented Dec 12, 2023

One solution that could stop applications from braking when attempting to connect to a wallet implementing this standard could be returning the cip Number in the supportedExtencions() api call if the selected wallet uses multi-delegation.

This way it becomes easy for dApp developers to simply reject multi-delegation wallets if the application does not support them.

@yHSJ
Copy link

yHSJ commented Dec 12, 2023

This way it becomes easy for dApp developers to simply reject multi-delegation wallets if the application does not support them.

Generally, most dApps won't "break" due to this change. For example, at JPG Store, we use the first index of the getRewardAddresses response to identify an "account". They have to sign with that stake key to authorize with our dApp and our transactions require that a signature from that key provides a witness. This is how many other dApps operate too.

But, the dApp would behave in a "strange" way for users with multiple stake keys. There would be assets that the wallet displays that dApps may not show, for example. Or change management could get funky, etc.

@leo42
Copy link
Contributor

leo42 commented Dec 13, 2023

Generally, most dApps won't "break" due to this change. For example, at JPG Store, we use the first index of the getRewardAddresses response to identify an "account". They have to sign with that stake key to authorize with our dApp and our transactions require that a signature from that key provides a witness. This is how many other dApps operate too.

Yes that is why is made the suggestion, an easy way for dApps to check if they are connecting to such a wallet will make all the3 controversy go away since dApp developers can now reject such wallets if the dApp does not support it.

@QuasarChains
Copy link

If it's left up to the wallets and the dapp builders I think we will see more problems in the long term. Composability should in my opinion always carry priority when designing and building.

I believe multi-delegation is important and is still a very unrealized opportunity to improve the stake pool network.

And how should DAOs or other groups or organizations that are using a multi-sig address that also have multi-delegation for that wallet going to be able to interact with dapps?

@AngelCastilloB AngelCastilloB force-pushed the multistake-delegation-from-single-account branch from 6eebaa3 to cdafdd4 Compare January 3, 2024 18:19
@AngelCastilloB
Copy link
Author

Hey @AngelCastilloB!

I have put this proposal on the Triage agenda for CIP Editors Call # 78 (December 12th 4pm UTC), held in The CIP Editors Meetings Discord, see agenda and discord event.

Would be great to see you there to discuss this proposal.

As we discussed during this meeting, I added a disclaimer to the document in the abstract:

While this proposal outlines a method for multi-delegation within the current framework of Cardano's features, it is acknowledged that more efficient strategies may exist, particularly if multi-delegation capabilities were to be integrated at the protocol level. This CIP offers a solution that operates effectively under the existing system. However, it is important to note that should the Cardano protocol evolve to natively support multi-delegation in the future, this CIP would be considered deprecated in favor of the more direct, protocol-level approach. Such an advancement would likely offer a more streamlined and potentially more robust method for managing multi-delegation, aligning with the evolving needs and capabilities of the Cardano ecosystem.

Let me know if we are happy with this @yHSJ @Ryun1 @rphair @rhyslbw et al

@AngelCastilloB
Copy link
Author

Thanks for submitting this! I left the following feedback in the Lace Discord, but felt I should share it here too:

My initial feedback would be that you should use RFC-2119 to strictly define the language use, and then modify the specification to use that language. This is feedback I've given to almost every CIP. Cardano's standards right now use a lot of ambiguous language that one could argue is implicitly defined, but as an implementer I've seen several different interpretations of standards. If RFC-2119 was used, that ambiguity is removed.

I tried my best to add this into the CIP, but I found it moistly made sense in the specification section, I will further improve it if needed tho

@AngelCastilloB
Copy link
Author

Generally, most dApps won't "break" due to this change. For example, at JPG Store, we use the first index of the getRewardAddresses response to identify an "account". They have to sign with that stake key to authorize with our dApp and our transactions require that a signature from that key provides a witness. This is how many other dApps operate too.

Yes that is why is made the suggestion, an easy way for dApps to check if they are connecting to such a wallet will make all the3 controversy go away since dApp developers can now reject such wallets if the dApp does not support it.

dApps can already do this by querying CIP-30 getRewardAddresses and checking if there is more than one. Some dApps have incompatibility issues with wallets having more than one stake key particularly, because by convention they are assuming wallets will only have one. This CIP is not introducing the ability for wallets to have more than one stake key, this is already a feature of the protocol, this CIP or basically any use of multiple stake keys will break these dApps.

In my personal opinion dApps shouldn't build around conventions but rather the protocol, this will ensure the dApps are robust to all use cases

@rphair
Copy link
Collaborator

rphair commented Jan 3, 2024

(#628 (comment)) Let me know if we are happy with this

Ordinarily a statement like this would go in the Rationale but I can admit it would be worth the couple inches of text to highlight it in the Abstract because it is an important qualification to the proposal's scope and target audience. If "@yHSJ @Ryun1 @rhyslbw et al" are offended by its presence there, it should be moved to the top of the Rationale.

@rhyslbw
Copy link
Contributor

rhyslbw commented Jul 6, 2024

Let me know if we are happy with this @yHSJ @Ryun1 @rphair @rhyslbw et al

LGTM @AngelCastilloB

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

The bump from @rhyslbw's #628 (comment) makes me think we should move this forward if there are no grounds for disqualifying it: so I've put it up for Review again at the next CIP meeting (https://hackmd.io/@cip-editors/92).

Most are in favour of doing this in the Ledger as per @AndrewWestberg #628 (comment), although the CIP editors haven't been made aware (as a group) in the meantime of any upcoming changes in the Ledger that would facilitate multi-delegation in the protocol.

  • @WhatisRT @lehins can we record here any relevant changes corresponding to Andrew's comment in the upcoming hard fork... or confirm that they're being considered for a future hard fork... or note here if they're not being planned at all?

I would be inclined to merge this as per what @yHSJ says about also having merged CIP-0018 (#628 (comment)) but I would definitely want to update it first about any Ledger changes or changing community expectations in the meantime.

Before merging I think we also need to see a review of any wallets besides Lace supporting this method as outlined (cc @Crypto2099 @Ryun1). I think it's OK to merge as Proposed given @AngelCastilloB's comment on the qualification added here (#628 (comment)) but the community & wallet / dApp developers might want to see a stronger statement (if true) saying (more or less): "This approach is taken in the Lace wallet, but nowhere else, and ideally it should be done in the Ledger."


### Acceptance Criteria

- [ ] The proposal is adopted by all major wallets and dApps in the ecosystem.
Copy link
Collaborator

@rphair rphair Jul 6, 2024

Choose a reason for hiding this comment

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

I think merging this as written would ensure that the CIP stays in Proposed status forever. Depending on the outcome of the discussion planned in my review above, this could be a sensible approach... although if other implementations are on the horizon then a more flexible acceptance threshold should be defined here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@AngelCastilloB - continued in #628 (comment) with a reason why it may actually make sense to leave this apparently unsatisfiable criterion on the list.

@rphair rphair requested review from Ryun1 and Crypto2099 July 6, 2024 09:55
@WhatisRT
Copy link
Contributor

WhatisRT commented Jul 8, 2024

I'm not aware of any plans to implement multi-delegation. Intersect would have to put that on the roadmap. There are a bunch of technical hurdles to this feature, since it touches some of the most complicated pieces of Ledger code. There are also no new features in Conway that should help implementing multi-delegation without ledger support.

@rphair rphair changed the title CIP-???? | Multi-Stake Delegation from a Single Account CIP-0126? | Multi-Stake Delegation from a Single Account Jul 14, 2024
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Thanks again for a constructive submission; please also retitle the containing directory as CIP-0126. 🎉

@AngelCastilloB @rhyslbw the CIP editors and community at our last meeting confirmed the need to qualify this proposal as "unique to Lace" — somehow, according to the recommendations above — before it is merged: to avoid institutionalising the potential conflicts with dApps that this alternative delegation approach might lead to.

CIP-XXXX/README.MD Outdated Show resolved Hide resolved
@AngelCastilloB
Copy link
Author

Thanks again for a constructive submission; please also retitle the containing directory as CIP-0126. 🎉

@AngelCastilloB @rhyslbw the CIP editors and community at our last meeting confirmed the need to qualify this proposal as "unique to Lace" — somehow, according to the recommendations above — before it is merged: to avoid institutionalising the potential conflicts with dApps that this alternative delegation approach might lead to.

Hi @rphair how should this be represented in the CIP?, not sure where to add this in the proposal, any preference/recomendation?, maybe we extend the disclaimer?

This proposal outlines a method for multi-delegation within the current framework of Cardano's features, and is currently unique to Lace wallet. It is acknowledged that more efficient strategies may exist, particularly if multi-delegation capabilities were to be integrated at the protocol level. This CIP offers a solution that operates effectively under the existing system. However, it is important to note that should the Cardano protocol evolve to natively support multi-delegation in the future, this CIP would be considered deprecated in favor of the more direct, protocol-level approach. Such an advancement would likely offer a more streamlined and potentially more robust method for managing multi-delegation, aligning with the evolving needs and capabilities of the Cardano ecosystem.

What do you think? ☝🏼

fix: address community feedback
@AngelCastilloB AngelCastilloB force-pushed the multistake-delegation-from-single-account branch from 191a9b0 to b38ef16 Compare July 22, 2024 12:35
@rphair
Copy link
Collaborator

rphair commented Jul 23, 2024

re: #628 (comment)

I think adding the term "currently unique to Lace wallet" - as you have done to the "disclaimer" paragraph in the Abstract - is both the best way of doing it (because it appears early in the document in a place where target audience is established) and a sufficient response to the community feedback from last meeting.

You could merge this paragraph into your document in this branch, so we can read how it appears in the regular flow... if not done before tomorrow's meeting (in 12 hours) we'll review it as above. If editors / devs think more is required than "unique to Lace" then we'll list the suggestions here after the meeting. cc @Crypto2099 @Ryun1

@AngelCastilloB
Copy link
Author

re: #628 (comment)

I think adding the term "currently unique to Lace wallet" - as you have done to the "disclaimer" paragraph in the Abstract - is both the best way of doing it (because it appears early in the document in a place where target audience is established) and a sufficient response to the community feedback from last meeting.

You could merge this paragraph into your document in this branch, so we can read how it appears in the regular flow... if not done before tomorrow's meeting (in 12 hours) we'll review it as above. If editors / devs think more is required than "unique to Lace" then we'll list the suggestions here after the meeting. cc @Crypto2099 @Ryun1

done in 89bff9a

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@Ryun1 @Crypto2099 I think we agreed at the last relevant CIP meeting that we would merge this as soon as officially documented as an approach unique to the Lace wallet. Since this did not trigger an "automatic" review I suppose therefore this just "fell through the cracks" so I'm tagging it Last Check for confirmation at (the next / a near-term) CIP meeting. cc @AngelCastilloB @rhyslbw

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Aug 20, 2024
@rphair
Copy link
Collaborator

rphair commented Aug 20, 2024

Note @Ryun1 @Crypto2099 it may in fact be better to leave the objectionable statement #628 (comment) in the CIP:

The proposal is adopted by all major wallets and dApps in the ecosystem.

... since that would in fact leave the CIP at Proposed state indefinitely... unless somehow the unconventional Lace standard were to become universal, and then could be made Active. Until such a time, this CIP still serves a purpose to document their own approach & potentially make itself available to other implementations. (cc @AngelCastilloB @rhyslbw)

CIP-0126/README.MD Outdated Show resolved Hide resolved
Co-authored-by: Ryan <44342099+Ryun1@users.noreply.github.com>
@rphair
Copy link
Collaborator

rphair commented Sep 17, 2024

@AngelCastilloB @rhyslbw marking Waiting for Author until reservation @Ryun1 already posted in #628 (comment) is met. Today's CIP meeting confirmed with community today that this is not admissible as a CIP unless it includes a forthright presentation about how & why this approach diverges from the prevailing approaches of other contemporary Cardano wallets.

@rphair rphair added State: Waiting for Author Proposal showing lack of documented progress by authors. and removed State: Last Check Review favourable with disputes resolved; staged for merging. labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Wallets Proposals belonging to the 'Wallets' category. State: Waiting for Author Proposal showing lack of documented progress by authors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.