axonweb3 / axon

Axon is a Layer 2 framework of CKB with native cross-chain and interoperability.
https://axonweb3.io
MIT License
65 stars 39 forks source link

`eth_getTransactionCount` returns non-monotonically increasing values #1604

Closed yangby-cryptape closed 9 months ago

yangby-cryptape commented 9 months ago

Description

Let's focus on the following code:

https://github.com/axonweb3/axon/blob/0457119f5bf51b311f3a7f5740c626485dd3a52c/core/api/src/jsonrpc/impl/web3.rs#L357-L369

This function does two things:

Notice that, there is a very very small gap time between the first action and the second second.

If any new block is committed in that gap time, what would be happpened? The result will become to $\alpha + (N + \alpha) = 2\alpha + N$.

So, in unit tests, when the code calls RPC methods eth_getTransactionCount(..., "pending") again and again. The result could be:

After the transaction with nonce 0x3a is committed, the all remained already-sent transactions after it would never be committed, since the nonce are not allowed to be decreased. These transactions will be stuck in mempool forever.

Proposal

I have no idea how to fix that base on current codes.

But I noticed that there is two fields named timeout_gap and timeout_config in the mempool. I don't know what does these fields do but they contains block numbers.

https://github.com/axonweb3/axon/blob/0457119f5bf51b311f3a7f5740c626485dd3a52c/core/mempool/src/pool.rs#L26-L27

So, maybe Axon could do the following changes:

  BlockId::Pending => { 
-     let pending_tx_count = self 
+     let (pending_tx_count, current_tip_number) = self
          .adapter 
          .get_pending_tx_count(Context::new(), address) 
          .await 
          .map_err(|e| RpcError::Internal(e.to_string()))?; 
      Ok(self 
          .adapter 
-         .get_account(Context::new(), address, BlockId::Pending.into())
+         .get_account(Context::new(), address, current_tip_number)
          .await 
          .map(|account| account.nonce + pending_tx_count) 
          .unwrap_or_default()) 
  } 

p.s. This issue is created via GitHub CLI.