IntersectMBO / cardano-ledger

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

Disallow empty treasury withdrawals #4582

Open lehins opened 2 weeks ago

lehins commented 2 weeks ago

Currently a TreasuryWithdrawal action can contain an empty map, which would result in a proposal that does nothing. This does not have any dangerous implications, but it doesn't make sense to have TreasuryWithdrawals that has no affect, especially since it has confused some developers in the past: https://github.com/IntersectMBO/cardano-cli/issues/876

We should add a predicate failure that prevents TreasuryWithdrawals with Maps that sum to 0 ADA. This would not only prevent empty withdrawals, but also the ones that don't try to withdraw any funds.

lehins commented 2 weeks ago

CC @WhatisRT and @williamdemeo I don't think we need to have a matching predicate check in the spec, but wanted to let you know that we are planning to implement a predicate failure like this.

WhatisRT commented 2 weeks ago

With these types of things I wonder if it'd be a good idea to implement some granularity for predicate failures. I.e. ones that affect block validation and ones that trigger when a node verifies individual transactions for the mempool, possibly functioning more like a warning. Then you could just add this to the next version without a HF. In general that would make it much easier to patch issues like that. You wouldn't get guarantees that these transactions don't exist, but that's not an issue for downstream tooling since it needs to handle the possibility of such situations in the era before they are fixed right now anyway.

lehins commented 5 days ago

With these types of things I wonder if it'd be a good idea to implement some granularity for predicate failures. I.e. ones that affect block validation and ones that trigger when a node verifies individual transactions for the mempool, possibly functioning more like a warning

@WhatisRT I've opened a ticket for a potential solution like this: #4622