Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: master
Are you sure you want to change the base?
CIP-0126? | Multi-Stake Delegation from a Single Account #628
Changes from 1 commit
2744bb2
cdafdd4
b38ef16
89bff9a
ae06747
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalization.
But also, how could this be expanded for DRep delegation? this is not clear to me.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea
Since it doesn’t fully attenuate I’d vote in favour of not enforcing unnecessary certificates.
#628 (comment)
There was a problem hiding this comment.
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 was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 the need for updating the metadata in the case where pools are swapped but no number of pools or percentages changes, agree.
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.
This is the same as point 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AngelCastilloB
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.
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There is no way to prevent consuming an UTxO from another wallet
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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