IntersectMBO / cardano-ledger

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

Potential unintended consequences for ref script fees #4594

Open twwu123 opened 2 months ago

twwu123 commented 2 months ago

Transaction fees are being charged for inputs with reference scripts, despite the script not actually being used, and the transaction having no redeemers. It's possible that it is intentional as it creates more work for nodes regardless of whether the reference scripts are used or not. However, from my testing, I don't think any wallets accounts for this.

This means that, it is currently completely possible to ruin someone's day by sending them a lot of random utxos containing ada of different sizes (to maximize the chance they get selected for), and attach some reference scripts to these utxos. This could essentially lock up their funds until the wallets push out a fix.

vsubhuman commented 2 months ago

Main unclear issue is why a tx is being charged for a non-plutus input having a ref script in it when NO Plutus execution and L2 is involved? It's a regular transaction with only vkey witnesses. In this case the node technically does not even need to care whether a regular input has any script in it or not, because it is completely irrelevant to the transaction execution.

lisicky commented 2 weeks ago

@lehins, could you tell me if it is possible to disable minFeeRefScriptsCoinsPerByte for transactions that don’t have any required script witnesses? Because it can easily affect any wallet just by sending a transaction with an inlined script to it. I’m pretty sure that wallet builders still don’t expect that case in their wallets, because it’s not just a casual use case, but this case might be a side effect of a dapp or a not-so-good third party.

lisicky commented 2 weeks ago

It also makes some UTXOs more expensive to spend if someone creates a UTXO with a 16kb inlined script for example

lehins commented 2 weeks ago

@twwu123, @vsubhuman and @lisicky you all have very good questions. More importantly, I have raised my own concerns that are very similar to yours when we were designing this feature. In the matter of fact, original implementation did exclude reference scripts that are unused. However, we had a lengthy discussion on the subject and unfortunately I lost the argument. @WhatisRT pointed out that it would be very complex to implement in the Agda specification and consequently convinced everyone including myself that this is not going to be an issue. After I did a bit of mainnet chain data analysis it turned out that at the time approximately 11.7 Mb or mere 0.064% of total PlutusV2 reference scripts are unused.

Honestly, I did expect that sooner or later someone will complain about it. So, thank you for raising your concerns. This makes me believe that we ought try and fix this for the next era, but I am not sure I'll be able to win the battle on my own, since I've already tried. I regret to inform you that there is nothing we can do for Conway in that respect. That's why I did not even comment on this issue earlier, since that ship has sailed for Conway even before this issue was created. However, if any of you is willing to submit a CIP that proposes this change of not charging for ref scripts that are unused then there is a better chance that it will be implemented for the next era.

I'll try and answer all of your questions and concerns:

It's possible that it is intentional as it creates more work for nodes regardless of whether the reference scripts are used or not.

It does not. So it is not intentional and there is no fundamental reason why we need to charge for reference plutus scripts that are no being used. There will be small exception in the future, however. When we add UTxOHD implementation then there will be slight overhead with fetching refscripts from disk, but the cost I suspect will be negligible when compared to deserialization.

Main unclear issue is why a tx is being charged for a non-plutus input having a ref script in it when NO Plutus execution and L2 is involved?

With respect to such inputs used as reference inputs it is not a problem, because the only reason why reference input should be included is to provide access to a script as a reference script. So, if a script is not used then such input should not be included in the reference inputs. However, with respect to inputs that are being spent, this is absolutely a valid point and this is exactly what this ticket is about.

could you tell me if it is possible to disable minFeeRefScriptsCoinsPerByte for transactions that don’t have any required script witnesses?

Nope, this is baked into the ledger rules and can't be changed in the era.

Because it can easily affect any wallet just by sending a transaction with an inlined script to it. I’m pretty sure that wallet builders still don’t expect that case in their wallets, because it’s not just a casual use case, but this case might be a side effect of a dapp or a not-so-good third party.

Unfortunately wallets will have to account for this use case. If the wallet uses cardano-api or cardano-cli to build transactions then it is already accounted for. Unfortunately, I can't speak for any wallets that don't use Haskell library that is provided by Intersect, they'll just have to do their own leg work on this one.

It also makes some UTXOs more expensive to spend if someone creates a UTXO with a 16kb inlined script for example

You are 100% right. Arguably, it will be also more expensive to create such UTxO, so there is little motivation to do so for the user that is creating such output.

CC @disassembler and @kevinhammond

twwu123 commented 2 weeks ago

Thanks for the response, although it's not exactly what I wanted to hear, it's good to know that the issue is at least acknowledged.

lisicky commented 1 week ago

Thanks @lehins for your detailed answer!

But I need to expand a bit on what I was saying earlier, because it has potential attack vector. Just to remind, we can have around 16kb of an inlined plutus script in transaction output per tx. Also can can have 400+ inputs in a pure tx. So if a tx already has 121 inputs with a 16kb referenced script it might cost 1928769771877 lovelace or 675,069.42 USD.

Let's assume we have a crypto exchange that just receives UTXOs and gives cash to a user in exchange for ADA. In general, everything is fine until the moment when the exchange starts selling ADA for cash and needs to spend all these UTXOs in one transaction. Or the exchange decides to reorganize all these UTXOs by merging them into one or moving funds to a specific wallet because it needs to. And what if all these utxo's has an inlined scripts. I guess you can already see the potential consequences. I wouldn't say it's 100% possible since I don't know the internals of a crypto CEX, but the chance doesn't seem too low to be impossible. It can also break a DEX's match maker or batcher by creating a set of orders that seem perfect to execute in one transaction. In general, if someone works closely with Cardano, this person will probably think about this to avoid such problems before they happen. But often people dont's care about anything till it happens or they were notified properly. The blockchain industry isn't limited to Cardano; there are plenty of platforms and numerous strange tokens that create additional informational pollution. This feature isn't something that was loudly announced, in contrast to all the marketing around governance stuff. If you work just with wallet non-Plutus script UTXOs, you don't expect anything that might affect fee calculation except transaction size. I had a conversation with a wallet builder (not Yoroi), and he doesn't expect such kinds of UTXOs in a wallet. And it's difficult to see the problem when you just receive such utxos, because it doesn't have some garbage assets that more visible and it explicitly increases transaction size when you are trying to spend it. I just googled "minFeeRefScriptsCoinsPerByte", and the first link is this one, where I see:

"The minFeeRefScriptsCoinsPerByte protocol parameter refines the Plutus cost model and improves fee calculations by allowing separate tuning of the costs for reference scripts."

But I hope that I'm just wrong, and cardano will not that problem.

lehins commented 1 week ago

@lisicky I appreciate your concern, but it is definitely not an attack vector. Let's go though some back of the envelope calculation:

It takes ~7.8 ADA to create an input that contains a reference script with 16KiB in size. Which means an attacker would have to pay approximately 93 ADA in fees for submitting 121 such transactions. Moreover, each input will have to contain at least 70ADA, due to min utxo requirement. Which means for an attacker to create 121 inputs with 16KiB ref scripts and send them to anyone would cost 8563ADA. That is already quite a bit of skin in the game. That being said, presumably, addressing those inputs to a CEX, would mean that 8470ADA would be exchanged for fiat or other crypto and the real cost would be only 93ADA. This is the amount we can use as cost basis of an attack.

So, now let's look at the receiving end. Let's say all of those 121 inputs where addressed to you (or to a CEX for that matter) then all you need to do spend one of them at a time (up to 25KiB of ref scripts per transaction) in order to minimize the cost. However, even if a wallet implements the dumbest thing possible and tries to spend all 121 inputs in one tx it will not work:

So if a tx already has 121 inputs with a 16kb referenced script it might cost 1928769771877 lovelace or 675,069.42 USD.

There is a limit of 200KiB of total reference scripts in a transaction, so at most it would be possible to include 12 such inputs. Hypothetically, a wallet would do that simplest, but not the smartest thing and include all 12 inputs like that in one transaction, then it would cost approximately 7 ADA extra for ref scripts in those inputs to spent such transaction. Doing that 10 time would lead to an extra cost of 70ADA for the CEX.

In other words loss of 19M ADA as you described is not possible and the cost for an attacker would higher than it to would be for a CEX/DEX that is being "attacked"

One might argue further that this is a bit unfair, and I would agree with it, as I pointed out in my earlier comment. But, because CEX or DEX will have to spend 7ADA extra for the minimum received 840ADA (= 12 * 70ADA), it is less than 1% of amount being transacted. Considering that anyone trying to abuse this would be at a loss, I believe, there is little to no concern of this becoming a problem for CEX/DEXs

I just googled "minFeeRefScriptsCoinsPerByte", and the first link is this one, where I see:

Here is a document that can give you a bit more insight into the parameter and motivation behind that parameter: https://github.com/IntersectMBO/cardano-ledger/blob/master/docs/adr/2024-08-14_009-refscripts-fee-change.md

This feature isn't something that was loudly announced, in contrast to all the marketing around governance stuff.

It was a security concern, as you will learn from the above document, so we could not be overly loud about this feature. Also, you comparison of this tiny feature to governance is a bit demeaning, because addition of Governance to Cardano is a huge milestone! It very much deserves all the marketing around it.

If you work just with wallet non-Plutus script UTXOs, you don't expect anything that might affect fee calculation except transaction size. I had a conversation with a wallet builder (not Yoroi), and he doesn't expect such kinds of UTXOs in a wallet.

I am sorry, but Cardano is not only about sending and receiving ADA and other tokens. It is much more than that, so expectations should be adjusted accordingly by such developers.

But I hope that I'm just wrong, and cardano will not that problem.

Hopefully my explanation and more information about this feature put your concerns at ease.

But often people dont's care about anything till it happens or they were notified properly. The blockchain industry isn't limited to Cardano; there are plenty of platforms and numerous strange tokens that create additional informational pollution.

Side note: this is not a good argument in my books at all. If one is willing to support a project they should invest time and money to do it right. Exchanges and wallets have the resources and motivation to do so. They really have to do their due diligence in understanding the product that they are claiming to support. If they are choosing to be sloppy, then they deserve to pay for it in exploits. This view that I have applies to any software out there, including Cardano!

WhatisRT commented 1 week ago

I should mention that it's not just about additional difficulty, it's also not clear to me that giving exceptions to this fee would be an improvement. The issue is that it's extra work (finding out that the fee is not necessary) that needs to be done on every transaction containing reference scripts. It's not much extra work of course, but it's making the common scenario more computationally expensive to make a rare scenario monetarily cheaper. The only use-case that I know where this fee applies is 'cleanup', and as @lehins argued there isn't really an attack here.

Also, note that checking whether there are no redeemers isn't good enough since the script could also be a native/phase 1 script. To know whether that's used, you have to actually look at all of the use sites, i.e. check with scriptsNeeded.

As for the argument that wallets need to deal with it, there are already some ways in which you can make an output look like it contains a bunch of money but it's not actually available: You can just include a large amount of garbage tokens that will block the min-ada amount for those tokens. If all of them are unique, there is no way of consolidating anything. Ideally a wallet would somehow show you that some of your ada is blocked by garbage tokens, but I don't think there exists a wallet that does that (and this issue was known well before the introduction of multi-assets).

However, I this this issue provides a good reason as to why the fee should grow linearly instead: If you're not an attacker and just want to clean up, you shouldn't have to worry about chopping up your transactions just to save on fees.

lehins commented 1 week ago

@WhatisRT We need to make sure we have our facts right. Computational and complexity overheads of this feature in ledger would be negligible! We already have scriptsNeeded available so we'd only need to do one single Map restriction in order to filter out unused scripts. It is literally a two line change. It is the complexity of changing specification that was driving factor behind us ending up charging for unused reference script.

With all that said, I do believe this would be an improvement and I recommend creating a CIP about this. Because without a proper CIP, I can guarantee that this change is not going to happen.

I this this issue provides a good reason as to why the fee should grow linearly instead: If you're not an attacker and just want to clean up, you shouldn't have to worry about chopping up your transactions just to save on fees.

@WhatisRT Same to you, if you don't like the way we are charging for ref scripts, feel free to create a CIP about it. Complaining on different tickets about the pricing model isn't gonna help it.