Fail early to consume gas if a user doesn't have enough balance

[robert-zaremba]

Summary of Bug

This is the highlevel overview of how the fee processing works today in DeliverTx:

1. When processing a transaction, we consume gas in middlewares and msg handlers
2. Gas is translated into fees, and fees are paid in https://github.com/cosmos/cosmos-sdk/blob/5d86db3/x/auth/middleware/fee.go#L191 . However:
   • if a user doesn't have enough balance, he won't pay anything
   • the problem is that we will waste resources and user won't pay anything for that
3. If a block is full, potentially the last transaction is unlucky: if the tx consume more gas than the block limit (we check it only after executing the tx, not as with fee payment - in the checktx), we don't persist the unlucky tx's state transitions (but the sender will still pay the fees).

Proposal:

Refactore Gas framework to:

• try to adjust the tx fee to the balance user has
  • if user has 1k uatoms, and fees are 1.1k uatoms, we should charge 1k uatoms..
  • adjust gas limit accordingly

Nice to have: #2150

Moreover:

• DeductFees should be private - it seams to was made public only for the sake of tests, which can be easily translated into the private tests. That function shouldn't be used outside of the middlewares.

Ref:

• fix: use full gas on overflow #10897
• fix: should revert tx when block gas limit exceeded #10770
• Refunding unused but allocated gas #2150

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[amaury1093]

i'm a bit confused tbh, because my understanding of your proposal is what we currently do.

• "set at the very beginning the max amount of gas we can use": ref, which is a top middleware.
• "early fail the TX if we use more gas": GasMeter panics, which means we fail as early as possible
• "charge as much fee for used gas as possible": we always deduct the tx.auth_info.fee. I don't think we should deduct more than that.

[amaury1093]

However, I do agree that if the user does not have enough balance to pay for fees, we should still charge the user something.

[alexanderbez]

if a user doesn't have enough balance, he won't pay anything, and his transaction will be cancelled
the problem is that we will waist resources and user won't pay anything for that

I fail to see how this is a problem really? This check happens during CheckTx and doesn't cost much in the grand scheme of things.

I would suggest restructuring the issue. The proposal doesn't like up with what you claim to be the issue.

[robert-zaremba]

The issue is not related to CheckTx but to DeliverTx phase. Sorry if that was confusing, in the issue I was trying to say that in CheckTx we do the necessary checks, but later, in edge cases, we don't charge the user correctly. I'm going to rephrase that in the issue.

[robert-zaremba]

Maybe, during this task, we should consider implementing "not used feed" (fee we paid for not used gas) return?

[alexanderbez]

@robert-zaremba this might interest you: #2150

[tac0turtle]

is this still relevant @alexanderbez ? with application based mempools i feel like most of this is solved, except the fee returning if overpaying.

[alexanderbez]

yeah I still don't quite follow the exact proposal. The main thing I get out of this is that we should perhaps charge a user something in the case where the don't have a sufficient balance to pay fees.