search

crypto-org-chain / cronos

Cronos is the first Ethereum-compatible blockchain network built on Cosmos SDK technology. Cronos aims to massively scale the DeFi, GameFi, and overall Web3 user community by providing builders with the ability to instantly port apps and crypto assets from other chains while benefiting from low transaction fees, high throughput, and fast finality.
Other
292 stars 236 forks source link

Cronos Chain disallows to replace transaction #423

Open ghost opened 2 years ago

ghost commented 2 years ago

Describe the bug Cronos chain disallows to replace transaction

To Reproduce Steps to reproduce the behavior:

  1. Send transaction with low gas price (simulate stuck)
  2. Try to send a transaction with the same nonce and higher gas price (speed up)

Expected behavior A transaction must be accepted

Additional context Error message from node ran with --trace flag:

{"jsonrpc":"2.0","id":1222,"error":{"code":-32000,"message":" --- at /go/pkg/mod/github.com/crypto-org-chain/ethermint@v0.7.2-cronos-6/app/ante/eth.go:222 (EthNonceVerificationDecorator.AnteHandle) ---\\nCaused by: invalid nonce; got 1688, expected 1689: invalid sequence"}}
JayT106 commented 2 years ago

The current dependency - Cosmos SDK v0.44.x is using Tendermint v0.34.14 doesn't support the priority mempool yet, the current mempool is FIFO. Therefore, require the upstream project to release v0.46.x for supporting this feature.

yihuang commented 2 years ago

The current dependency - Cosmos SDK v0.44.x is using Tendermint v0.34.14 doesn't support the priority mempool yet, the current mempool is FIFO. Therefore, require the upstream project to release v0.46.x for supporting this feature.

I think even with tendermint 0.35, we still don't have the replace feature, right? In ethereum one can replace a tx in the mempool with the same nonce, to update the gas price for example.

JayT106 commented 2 years ago

If we update the gas price in the transaction, It will affect the txhash, right? So I guess what happens is the updated transaction will be executed first (if the App applied higher tx priority), and the original transaction should be rejected later (from the app). In tendermint's mempool design, it doesn't care about the gas price and nonce. So the logic should be handled on the App side.

But I need to check what happens if two txs have been proposed in the same block.

JayT106 commented 2 years ago

So, for the replace transaction, it requires ABCI++ PrepareProposal to support it. So the App can drop the original transaction when the block has 2 transactions everything is the same except the gas price.

In TM 0.35, the best tx processing will be if 2 transactions are proposed in the same block, the one that has a higher priority will succeed and the other one will be failed. And if one transaction has been executed first, so the App can drop another one in the mempool when calling reCheckTx (this mempool config need to be setup true)

yihuang commented 1 year ago

let's wait for the new mempool implementation in cosmos-sdk: https://github.com/cosmos/cosmos-sdk/pull/13262