IntersectMBO / cardano-ledger

The ledger implementation and specifications of the Cardano blockchain.
Apache License 2.0
257 stars 158 forks source link

Backwards compatibility for ConwayWdrlNotDelegatedToDRep #4092

Closed Quantumplation closed 1 month ago

Quantumplation commented 8 months ago

Can we get increased clarity on the exact semantics of the ConwayWdrlNotDelegatedToDRep rule?

In particular, how does it interact with script withdrawal credentials, and in particular those from previous eras?

Many dApps depend on the ability to "withdraw" 0 assets to delegate logic to a staking script and save on execution units. There is some worry that the new "must be delegated to a dRep" could break Plutus v1 and Plutus v2 scripts, as they are not able to delegate to a dRep, and thus would be immediately unable to execute a withdrawal, locking potentially millions of dollars of user funds with the Conway hard fork.

Will delegatedAddrs from here include registered script credentials? If not, I believe the proposed implementation suffers from the concern above.

I believe we need to make this condition either:

To avoid breaking backwards compatibility.

Quantumplation commented 8 months ago

(as a side note, I'm very proud of myself for navigating the Haskell code base well enough to find where this condition was implemented LOL)

JaredCorduan commented 8 months ago

well spotted @Quantumplation !

Quantumplation commented 8 months ago

@JaredCorduan It was actually someone else, Riley (@ILikeCardano) on Twitter, afaik. I'm just the one who thought to bring it to Github to ask about it, rather than arguing in a twitter space :P

WhatisRT commented 8 months ago

Oof, I wasn't aware of this trick/abuse (classic https://xkcd.com/1172/). I'll need to think a bit about this, but obviously we'll have to do something about it. Here's another hacky way to solve this: just always allow withdrawals of 0 coins. Automated delegation seems bad, because you can't delegate without having a credential registered first, so adding this would potentially impact other parts of the system as well.

Quantumplation commented 8 months ago

heh, yea, it turns out this trick can save an absurd amount of execution units, so it's been heavily used at this point.

Exempting withdrawal 0 is a partial solution, but might still catch some people unawares; With the Sundae contracts, for example, we explicitly fail when attempting to delegate so that this staking address can never earn rewards, but others may not have done the same thing.

We were discussing it on twitter, and probably the simplest solution is to filter script credentials out of wdrlCreds here.

That would allow script credentials to withdrawal without delegating, without undermining the goal of this rule: If the goal is to get an acknowledgment of governance from people, or an explicit opting out, then creating a proxy stake script to avoid delegating to the abstain DRep seems to serve the same role of "explicit opt out", and will only be an option for the most dedicated of technical users heh.

jmhrpr commented 8 months ago

With the Sundae contracts, for example, we explicitly fail when attempting to delegate so that this staking address can never earn rewards, but others may not have done the same thing.

As an aside, and just in case you hadn't considered, there are probably other ways for rewards to end up in your stake address, like someone specifying your stake address as a reward account for a pool perhaps?

Quantumplation commented 8 months ago

@jmhrpr Good to know! We hadn't thought of that case in particular, but we did think of this for MIR transfers by allowing a withdrawal of non-zero rewards in our scripts; it's just pretty inconvenient because most transaction builders aren't going to monitor the reward amount and build the correct tx, so at worst it'd cause a small hiccup until someone did withdraw the rewards.

WhatisRT commented 8 months ago

Yeah, there's an even easier way in Conway: when proposing a GA, you can specify any stake address to return the deposit to. It's probably worth building a system that doesn't suffer when there's money in the reward account.

We could just exclude scripts in general, I don't think there'd be an issue with this. However, it's a bit inconsistent and I'm not sure people even care about this, so maybe we can just get rid of it entirely.

lehins commented 7 months ago

It sounds like there are only two sensible solutions here:

After some discussions with various parties it seems like the former is the best path forward, since that still takes care of the common case of a regular user being forced to delegate to a DRep, while not affecting power users that rely on scripts for staking.

Any objections in going with this solution?

Quantumplation commented 7 months ago

Nope, I don't see any issues with that!

That would effectively equate to just filtering ScriptCredentials out of wdrlCreds here right?

lehins commented 7 months ago

That would effectively equate to just filtering ScriptCredentials out of wdrlCreds here right?

Yep, that's exactly it

zygomeb commented 5 months ago

It sounds like there are only two sensible solutions here:

  • either prevent withdrawals for only for KeyHash stake credentials that have not delegated to a DRep that are. This would essentially preserve the current behavior for PlutusV1 and PlutusV2 scripts
  • or drop the feature completely

After some discussions with various parties it seems like the former is the best path forward, since that still takes care of the common case of a regular user being forced to delegate to a DRep, while not affecting power users that rely on scripts for staking.

Any objections in going with this solution?

Would this feature be retained in PlutusV3? That would be desired.

lehins commented 5 months ago

Would this feature be retained in PlutusV3? That would be desired.

@zygomeb None of the scripts will be affected by this feature, that includes PlutusV3 onwards

OptimGMI commented 4 months ago

@lehins confirming that upcoming node 9.0 will have have this feature included?

https://x.com/IOHK_Charles/status/1799817186204057922

Need compatibility or we risk bricking all of DeFi

lehins commented 4 months ago

Thus feature does not affect 9.0 but it will 10.0, because that is when we leave the bootstrap phase.And yes, I can confirm that plutus scripts will not be affected. So, no worries, no bricking of DeFi will be allowed.