Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions are not guaranteed to run according to their arrival order #699

Open
m-Peter opened this issue Dec 10, 2024 · 8 comments
Open

Comments

@m-Peter
Copy link
Collaborator

m-Peter commented Dec 10, 2024

Currently, for each transaction submitted with eth_sendRawTransaction, we wrap an EVM.run call within a Flow transaction, sign it, and submit it in the Flow network using an Access Node.

If 2 transactions A, B are submitted with a very slight delay (say milliseconds), there's a chance that the Flow transactions that wrap EVM transactions A, B get executed in a reverse order. This is due to the pipeline architecture of Flow. The Flow transactions for A, B might be hashed to different collections, and the consensus node might choose to run B before A.

For the most part, transactions A, B might be unrelated to each other, but if they originate from the same EOA, then transaction B will fail with a nonce too high error, while transaction A will succeed.. There might be other cases where the reversal of transaction execution affects the end result.

One possible solution to this, is to have a TxPool, that will accept incoming transactions, and use EVM.batchRun, to execute them according to their arrival order. In order for this to work properly, we'll have to fine tune the amount of period a transaction should stay in the TxPool, before being submitted with EVM.batchRun.

Related issue: #477

Appendix: An example of a Flow block on testnet, where there's 3 failing EVM transactions from the same EOA: https://testnet.flowscan.io/block/231321798.

@bluesign
Copy link

I am not sure if batchRun will fix this. is it panic safe?

@m-Peter
Copy link
Collaborator Author

m-Peter commented Dec 11, 2024

I am not sure if batchRun will fix this. is it panic safe?

@bluesign Here you can see a usage of EVM.batchRun in action:

transaction(encodedTxs: [String]) {
    prepare(signer: auth(Storage) &Account) {
        var txs: [[UInt8]] = []
        for enc in encodedTxs {
            txs.append(enc.decodeHex())
        }

        let txResults = EVM.batchRun(
            txs: txs,
            coinbase: EVM.EVMAddress(bytes: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19])
        )
        for txResult in txResults {
            assert(txResult.status == EVM.Status.successful, message: "failed to execute tx, error: ".concat(txResult.errorCode.toString()))
        }
    }
}

It accepts an array of RLP-encoded transactions, executes them in their given order, and returns the execution result. It does not panic or anything like that, it allows the caller to decide what to do, based on the result of each executed transaction.
Do you have any specific concern that this approach is not going to fix the tx ordering issue?

@bluesign
Copy link

bluesign commented Dec 11, 2024

@m-Peter sorry I was too brief in first message, what I meant was; what if batchRun rans out of cadence computation and panics.

afaik there is no direct relationship with gas and cadence computation, how we will batch the transactions ? Even if there was direct relationship, can't someone send a very high gas limit transaction, but only pay for what is executed?

@m-Peter
Copy link
Collaborator Author

m-Peter commented Dec 11, 2024

@m-Peter sorry I was too brief in first message, what I meant was; what if batchRun rans out of cadence computation and panics.

afaik there is no direct relationship with gas and cadence computation, how we will batch the transactions ? Even if there was direct relationship, can't someone send a very high gas limit transaction, but only pay for what is executed?

That's some good points 👌

  • So the Flow transaction that wraps EVM.batchRun doesn't have any other operations, so we would likely have to set the max transaction limit for it.
  • I believe that there is a relationship bet EVM gas & Cadence computation. Something like 3000 EVM gas for 1 Cadence computation. But @janezpodhostnik might know better about this.
  • In theory, I believe that 1 Flow transaction, can handle up to 120_000_000 EVM gas, at least that's what I recall from Ramtin. The goal here is not to pack 10-20 EVM transactions in EVM.batchRun, but more like 4-5, and especially the ones that originate from the same EOA, and came in very close proximity.

Definitely it needs more thinking, on how to tackle this 🙏

@zhangchiqing
Copy link
Member

zhangchiqing commented Dec 11, 2024

For the most part, transactions A, B might be unrelated to each other, but if they originate from the same EOA, then both of them will fail, as there will be a nonce mismatch for both of them.

Just to point out one detail, I think in this case, transaction A might still succeed.

For instance, if account A nonce is 3, and transaction A is to use nonce 4, transaction B is to use nonce 5. If they are wrapped in 2 flow transactions and send B after A, then yes, there is still a chance B will be executed before A. In that case, B will fail, because nonce 5 is not the next nonce, but A will still succeed, because 4 is the next nonce.

@janezpodhostnik
Copy link
Contributor

Something like 3000 EVM gas for 1 Cadence computation.

That is correct.

Here is an example on how it works:

  • lets say FVM is executing a cadence transaction with computation limit 1000
  • The transaction used up 150 computation so far
  • An EVM transaction is encountered with a gas limit of 300.000
  • FVM checks if there is enough computation left 1000 - 150 > 300.000 / 3000 850 > 100
  • because there is it gets executed but only 30.000 gas was used. so only 10 computation is deducted.
  • The remaining computation is 850 - 10 = 840

@bluesign
Copy link

because there is it gets executed but only 30.000 gas was used. so only 10 computation is deducted.
The remaining computation is 850 - 10 = 840

I don't think this is correct considering storage read and writes. ( I complained on this too many times, let me explain a bit )

Here I have 2 successful gateway transactions, which are pretty similar except the gas usage.

let's take this transaction as example: ( which has 151 execution effort )
https://www.flowscan.io/tx/1af3d88333e1971ca4ca4c55be8d4fb996afe6a7cbd1133d0f249568995e46e9

This has 2 EVM transactions: ( one for main transaction ( 38013 gas ) , and 21000 gas for coinbase transfer )

So 38013 + 21000 = 59013 total gas used ( this is predictable as we have coinbase transfer always 21000 )

59013 / 30000 = 1.967 ~= 2 computation

now this has overhead of ~150 computation ( which matches hypothetical example from Janez )

now let's another example with higher execution effort: ( which has 356 execution effort )

https://www.flowscan.io/tx/d3b63de19c2f7d7dbd56a806d8a9acebc81d5885a5d2de8a809b20078e1a098e

120913 + 21000 = 141913 total gas used for this one

141913 / 30000 = 4.73 ~= 5 computation

Now we would expect this would cost ~153 computation, but now this has difference of ~203 computation ( difference is more than our prediction )

So gas usage cost is translating to Flow computation effort:

you can simply check with opcode costs also:

SSTORE non-zero to non-zero ( most common probably ) ~= 5000 evm gas = 5000 / 30000 ~= 0.16 cadence computation ( which gives us around ~13 bytes of storage read/write ) how atree with inlining works here has much more cost.

I think when adjusting execution efforts in the future, EVM also will need some work to make it more predictable.

There were edge cases when initial computation limit check success but EVM.run can run out of computation while executing. ( when I was developing COA Attachment I hit this before )

PS: technically now transactions on Flow are almost free and this is not an issue for now.

@bluesign
Copy link

One possible solution to this, is to have a TxPool, that will accept incoming transactions, and use EVM.batchRun, to execute them according to their arrival order. In order for this to work properly, we'll have to fine tune the amount of period a transaction should stay in the TxPool, before being submitted with EVM.batchRun.

I think instead of waiting maybe we can rate limit send by account address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants