Require native-asset fee notes when fees are enabled - #2564
Mirko-von-Leipzig wants to merge 8 commits into
Conversation
| if note.recipient().script().root() != fee_script_root { | ||
| continue; | ||
| } | ||
| if !matches!(note.assets().as_slice(), [asset] if asset.id() == fee_asset_id) { |
There was a problem hiding this comment.
I think it's technically possible to create a valid fee note that has the assets, but with a quantity of 0. If so, wonder if we should check that the amount is at least > 0 (but maybe it'd be even better to have a minimum amount that a minimal transaction would cost to compare against). cc @PhilippGackstatter @mmagician to confirm
There was a problem hiding this comment.
I actually want to allow zero so that this same flow works on networks with a zero fee.
This technically doesn't work at the moment because some protocol procedures explicitly emit no note when fees set to zero.
It's an.. open question at the moment
There was a problem hiding this comment.
I think none of the standard auth components emit fee notes when fees are zero. I think the main motivation was avoiding touching most protocol tests that would now output fee notes and change the assertions. Certainly something we could revisit, but for now the node may be better off skipping the fee requirement when fees are zero.
There was a problem hiding this comment.
Updated in bb18069. Both standalone transaction endpoints now allow a missing fee note when the transaction reference block has a zero verification base fee. Any fee notes that are emitted must still contain only the native asset; user batches remain exempt. The batch-building PR (#2647) also skips the collector transaction when there are no fee notes, since a deployed collector cannot execute without input notes.
There was a problem hiding this comment.
But should transactions that create a fee note with 0 assets (or a small enough amount of tokens which you know won't cover the tx) for networks that require fees even be allowed? Technically yes because at the batch level fees may still be enough to include the transaction? I understand standards pay fees correctly but a user could decide to pay whatever amount of fees they wanted.
There was a problem hiding this comment.
The batch builder can decide, in theory, to sponsor or accept whatever transactions it wants.
I wanted this to be a flat:
- never allow txs without a fee note
- only accept fee notes with a native asset
- only accept fee notes which pay more than their tx requires
but we can' really do that
| if note.recipient().script().root() != fee_script_root { | ||
| continue; | ||
| } | ||
| if !matches!(note.assets().as_slice(), [asset] if asset.id() == fee_asset_id) { |
There was a problem hiding this comment.
I think none of the standard auth components emit fee notes when fees are zero. I think the main motivation was avoiding touching most protocol tests that would now output fee notes and change the assertions. Certainly something we could revisit, but for now the node may be better off skipping the fee requirement when fees are zero.
d57c110 to
4d1986a
Compare
Standalone transactions are now rejected if they omit a
TX_FEEnote when the reference block's verification base fee is nonzero. Any fee notes they emit must contain only the native asset. This asset restriction will be relaxed in the future.Transactions can omit fee notes when the reference block's verification base fee is zero. Zero-valued native fee notes are also accepted. These checks do not determine whether the amount covers the transaction cost.
User-submitted batches are exempt. They are already proven and handle their own fee collection, so their transactions do not each need to output a fee note.
Part of #2501.
Changelog