stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

[chainstate] deduct fee before running the transaction

Open jcnelson opened this issue 2 years ago • 14 comments

Right now, we deduct the transaction fee after running the transaction. This can lead to problematic transactions which run to completion successfully, but where the sender or sponsor doesn't have enough STX to pay the fee. The inclusion of these transactions invalidate the block, so miners are forced to run these transactions to completion before they know not to include them. #3212 implements a mitigation in a backwards-compatible way, but ideally we would simply bill transaction fees before running transactions in the first place. However, doing so is a consensus-breaking change.

jcnelson avatar Jul 22 '22 22:07 jcnelson

One option is to make it so that if you get an ::InvalidFee error, we could roll back the tx and just debit the fee and materialize no other changes. This is similar to how Ethereum works. But, we'll want to ask smart contract devs how they feel about this change. Note that wallets could still determine that a transaction is likely (or definitely will) hit this case, because Clarity can be statically analyzed.

jcnelson avatar Jul 25 '22 15:07 jcnelson

There are basically two ways we can make sure miners can collect the fee even if the transaction's sender runs out of STX while the transaction is being processed:

  • Option 1:

    • Deduct the transaction fee from the sender. If this fails due to a lack of STX, then the whole block is invalid (i.e. these transactions are never mined).
    • Process the transaction payload. If this fails, then all changes are rolled back, but the transaction is mined and the fee remains deducted.
  • Option 2:

    • Check to see that the sender has enough STX to pay the fee. If not, then the whole block is invalid (i.e. these transactions are never mined).
    • Process the transaction payload. If this fails, then all changes are rolled back, but the transaction is mined and the fee remains deducted.
    • Deduct the fee from the sender. If this fails (e.g. suppose the transaction spent all the sender's STX), then all changes are rolled back, the fee is deducted from the sender's account, and the transaction is mined.

The difference between these two options is seen in the following example.

Suppose Alice wants to register five BNS names at once. To do so, she sends a smart contract transaction that preorders them all in a (begin) block:

(begin
   (if (is-err (contract-call? SP000000000000000000002Q6VF78.bns name-preorder 0x399c7f0cf9f90d08da6c96c605a9166c2c2f5138 u1000000))
      true
      true
   )
   (if (is-err (contract-call? SP000000000000000000002Q6VF78.bns name-preorder 0x6248ebcf673117980830191b06000a3a7eea7311 u1000000))
      true
      true
   )
   (if (is-err (contract-call? SP000000000000000000002Q6VF78.bns name-preorder 0xfb8b09d8f56532776ca6f07473522e185f8a094b u1000000))
      true
      true
   )
   (if (is-err (contract-call? SP000000000000000000002Q6VF78.bns name-preorder 0x13a8a3ac65155072b680973ac1b9fec5f4e64525 u1000000))
      true
      true
   )
   (if (is-err (contract-call? SP000000000000000000002Q6VF78.bns name-preorder 0x8955ce2197f58580a88aa6f3544a1c88768801a7 u1000000))
      true
      true
   )
)

As you can see, she's spending 5 STX (5,000,000 uSTX) in total. So, suppose she loads up her wallet with exactly 5 STX. But, suppose that she pays a transaction fee of 0.1 STX, meaning that she actually needed 5.1 STX total in her wallet.

Today, this transaction would not be mined at all. Alice's transaction would run to completion, and when the software tries to deduct the fee from her account, it fails for lack of STX. When a miner discovers this, it omits the transaction from the block entirely (because its presence in the block would invalidate the block).

With Option 1, the transaction gets mined, and Alice successfully preorders the first four names. The fifth one fails, because she only has 0.9 STX available.

With Option 2, the transaction gets mined, but none of the preorders materialize since Alice didn't have enough STX to pay the fee at the end.

My ask to the community is this: which fee-processing option would you like to see in Stacks 2.1? Please emoji-vote:

  • :+1: for Option 1
  • :smile: for Option 2

jcnelson avatar Jul 26 '22 18:07 jcnelson

option 2 may be better from apps sending transaction as they don't have to track which parts of previous transaction were successful and which weren't.

jiga avatar Jul 26 '22 19:07 jiga

@jiga Smart contract authors today already have to deal with the possibility that any asset-moving operation could fail for lack of sufficient funds (Clarity doesn't let you avoid handling the error case), which is why it's not clear to me why "they don't have to track which parts of previous transaction were successful and which weren't" is a win -- they're already tracking this, and implementing Option 2 doesn't change that requirement.

jcnelson avatar Jul 27 '22 01:07 jcnelson

@jcnelson option 1 sounds more intelligent and expected behaviour to me - especially if this were say a bulk transfer of 200 NFTs and it just fails because of the 199th.

Are there complexities though that the miner makes this calculation but Alice is running other transactions concurrently, which affect her balance and the outcome of the miners calculation? Are there different technical risks associated with implementation of each option?

radicleart avatar Jul 27 '22 07:07 radicleart

@jiga Smart contract authors today already have to deal with the possibility that any asset-moving operation could fail for lack of sufficient funds (Clarity doesn't let you avoid handling the error case), which is why it's not clear to me why "they don't have to track which parts of previous transaction were successful and which weren't" is a win -- they're already tracking this, and implementing Option 2 doesn't change that requirement.

I think what @jiga is saying is related to the fact that right now the operation is either successful or failed. With the first implementation, it gets partially successful as some things happened but not the intended action ( to mint exactly 5 bns ).

From an app dev perspective, it is cleaner to know if happened exactly what was intended or if it failed. Let's say instead of having 5 claims of the same type the contract call is a wrapper for other 5 claims each with 1 stx from different contracts ( also applies with 5 bns calls, gets harder with different contracts in the app implementation ). In my app after you claim those 5 you get access to something. If it fails (because of fees) you get nothing. What happens if it only claims the first 2, 3 or 4?

  1. Make the user claim again all of them and the user lost what was successfully spent
  2. Create extra cases that take into consideration every single option from the place it claimed successfully and continue specifically from there

BowTiedDeployer avatar Jul 27 '22 11:07 BowTiedDeployer

@BowTiedDeployer The handling of partially failed sequences of contract calls (fails after first 2 or 3 or so) is the same with option 1 and 2.

The question is more about whether to deduce the fees from the users balance first and the execute as far as possible, or to execute as far as possible and then fail everything due to low balance.

For the case of the 5 claims, if we assume that the sequence of 5 claims needs to be executed the only difference would be different error codes. Option 1) would generate a custom error code from your function because only 4 of the 5 claims succeeded. Option 2) would generate an invalid fees error because the 5 claims succeeded but there were not enough stx for the fees.

I am for option 1) because it gives the developers the option to make use of as much stx as possible from the user. Also, a function call to wipe a wallet would be able to determine the correct value to transfer:

Assume the wipe call costs 0.1 STX and the user has a balance of 5 STX before the call. With option 1) the wipe function checks the balance and sees 4.9 STX, the call would succeed. With option 2) the balance would be 5 STX and the tx would fail.

friedger avatar Jul 27 '22 13:07 friedger

I'm for option two @friedger I think the smart contract dev can add a checker first if possible where the function checks if the user has enough funds to execute Option 2 is also clearer and aligns with the philosophy of post conditions where what you see is what you pay, with the current stacks implementation it would suck to send another transaction but it is still a much much clearer way

hozzjss avatar Jul 27 '22 17:07 hozzjss

As @friedger points out, one consequence of Option 2 from a contract implementation standpoint is that the contract doesn't know how much STX to leave in the account to pay the fee (since the fee isn't exposed to Clarity). This could lead to some surprising transaction failures for users who don't have quite enough STX to do what they want to do. Wallets would have a hard time knowing what fee to set, since the contract could spend an arbitrary amount of STX in ways that can't be predicted in advance. While it's possible to expose the fee as a Clarity global variable, there's no way to force contract authors to use it (and it doesn't help you if the STX spend is dependent on data that isn't known at the time the tx is submitted). Option 1 does not have this problem, since the fee is paid in advance.

jcnelson avatar Jul 27 '22 17:07 jcnelson

It seems the major argument for Option 2 is that it provides "all or nothing" semantics for transactions. In this example, only four of the five preorders would get mined.

I'd like to remind folks that you still have this in Option 1 -- all you'd need to do is wrap any STX transfer or burn with unwrap-panic. Then if the STX transfer fails due to insufficient funds, the whole transaction aborts. In my example snippet, Alice would simply wrap the contract-call?s to .bns with unwrap-panic to get these semantics (which is less verbose than what I have here).

jcnelson avatar Jul 27 '22 17:07 jcnelson

Options 2 is more along the current implementation, i.e. that the fees are deducted after the transaction and the the contract sees the full stx balance before deducting the fees.

@hozzjss This is not about sending more txs, but about the stx balance that is available in the tx.

friedger avatar Jul 27 '22 17:07 friedger

Alright I’m convinced opt out is better

hozzjss avatar Jul 28 '22 02:07 hozzjss

Option one seems like the more sane choice. Friedger's balance wipe is the perfect example. Without a new constant exposing the transaction fee it will be rather tricky to pull it off with option two.

MarvinJanssen avatar Jul 28 '22 09:07 MarvinJanssen

Option 1 seems simple enough and straightforward. So the only difference between right now and Option 1 is the switch between the first step and the second step (more or less), correct?

fiftyeightandeight avatar Jul 29 '22 09:07 fiftyeightandeight