Improve fee deduction on errors

[ethanfrey]

From
#323 (comment)

With the current setup (anti-spam), insufficient fees cause rejection in CheckTx and never make it to a block. If it does make it into a block, all fees are deducted (assuming the fee account owner has signed) before executing any transactions, and are not rolled back on failure. The entire amount of fees are deducted in any case.

Before working out product fee behavior, maybe we should consider if the above is optimal.

Another approach:

On error, we only ever deduct the minimum anti-spam fee (as we rollback all work)
On success, we deduct the entire fee amount sent with the transaction
Not sure if it is better. It is "nicer" to people who made mistakes, but maybe more open to abuse. It is also more compatible with what we would desire from product fee behavior, I believe.

---

And a proposed technical implementation mentioned here
#323 (comment)

But on reflection, I would propose another approach (feedback welcome)

Fee decorator...

• deduct minimal fee
• start cache wrap
• deduct full fee
• on error, discard cache and return
• perform tx
• on error, discard cache and return
• check if required fee greater than paid fee
• if not, discard cache and return an error
• commit cache and return result

On error or insufficient fee, only anti spam fee will be deducted and no other changes will be writen.

If TX is writen, we guarantee entire required fee was deducted.

(note required fee section is part of #349)

[ethanfrey]

Note: I would make a new FeeDecorator (DynanmicFeeDecorator?) with this behavior, rather than changing the current one, as this also affects the use of Savepoint Decorators, and needs to be consciously changed in the calling code.

[husio]

It is hard for me to understand the functionality that is expected here. Reading the related discussion provides only a small update.

I understand that we want to implement a decorator that will be adding a fee to each transaction depending on that transactions type. Fee value differs between transaction types and it is declared and stored in the database.

An open question and a design decision are how to deal with fees when transaction processing fails. For the spam-fee we always want to collect it regardless the transaction success. For the transaction specific fee, we only want to charge it if the transaction was accepted and successfully processed.

To fulfill the requirement of canceling the transaction specific fee on failure, we must collect both fees separately. This I believe is described in a comment about RequiredFee attribute. Should CheckResult and DeliverResult be extended with the RequiredFee attribute?

Do we need RequredFee attribute? If we cache database operations and can rollback/commit them atomicaly, why not always collect transaction specific fee and let them be rolled back together withthe transaction?

I would like to write a rough implemnetation but I do not undestand enough to write the code.

[ethanfrey]

To fulfill the requirement of canceling the transaction specific fee on failure, we must collect both fees separately. This I believe is described in a comment about RequiredFee attribute. Should CheckResult and DeliverResult be extended with the RequiredFee attribute?

Yes, this was pulled into another issue #349

This issue just focuses on the rollback part, which works with current fee setup.

One could eg. send 1 IOV, while the min fee is only 0.1 IOV.
On success, you pay the 1 IOV, but we can just deduct the minimum 0.1 IOV if your transaction failed.
This is "nice" when fees are optional, but very much important to the user if we have product fees, and when you pay 20 not 21 IOV for a feature, you shouldn't lose the whole payment, but rather just the anti-spam fee cost (since nothing got writen/no feature used).

This is a prep step for the other.

I will prepare a rough PR (no tests...) to show roughly when I mean, and you can take it from there. Maybe that is the clearest way to communicate intent.