hashgraph / hedera-services

Crypto, token, consensus, file, and smart contract services for the Hedera public ledger
Apache License 2.0
299 stars 134 forks source link

Wrongful nonce error when sending transaction from lazy-created account #9310

Closed konstantinabl closed 5 months ago

konstantinabl commented 1 year ago

Description

Sending a transaction from a new lazy-created account that reverts and then we send a second one, the second one fails with WRONG_NONCE, although the sent nonce and the account nonce in hash scan and requested via the relay both appear as 0 In the relay the error is Nonce too low. Provided nonce: 0, current nonce: 0

The issue was found when investigating https://github.com/hashgraph/hedera-json-rpc-relay/issues/1803

Steps to reproduce

  1. Create a lazy-created account (I did it by sending 50 hbar to a wallet account created via ethers)
  2. Use the private key of this lazy-created account to sign a transaction for contract creation, where the gasPrice is not enough - the transaction is a legacy one of type: 0
  3. Send this transaction via sendRawTransaction endpoint of the relay
  4. A hash is received but the transaction actually failed with INSUFFICIENT_TX_FEE
  5. Change the gasPrice and send the transaction again, it fails with Nonce too low. Provided nonce: 0, current nonce: 0

Additional context

No response

Hedera network

testnet

Version

0.42.1

Operating system

macOS

mshakeg commented 11 months ago

Hey @konstantinabl just wanted to ask if you have any idea when this will be fixed?

mshakeg commented 11 months ago

Hey @Nana-EC just wanted to ask if there's any indication of progress on this issue, would really prefer to not deploy a non-canonical version of safe which is required for our dApp.

Nana-EC commented 11 months ago

Hey @Nana-EC just wanted to ask if there's any indication of progress on this issue, would really prefer to not deploy a non-canonical version of safe which is required for our dApp.

Hey @mshakeg will circle back this week on status or when we may get to this.

stoqnkpL commented 11 months ago

Hi @mshakeg I'm really not sure what's going on here. That error for sure looks incorrect and doesn't make sense: Nonce too low. Provided nonce: 0, current nonce: 0 but I'm having a hard time reproducing it. As far as I understand, it was reproducible with the steps above, but we tried on local-node and testnet environments and it no longer is. Can you try again and share the exact steps if you're able to reproduce the issue?

P.S. we didn't test on mainnet, only testnet and local node.

mshakeg commented 11 months ago

Hey @stoqnkpL hmm, that's really strange, but the issue still persists for the below testnet transaction by account 0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37

curl -X POST --data '{"jsonrpc":"2.0","method":"eth_sendRawTransaction","params":["0xf8a88086019c1c38a400830149f88080b853604580600e600039806000f350fe7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe03601600081602082378035828234f58015156039578182fd5b8082525050506014600cf3820273a0643f821328556c0a64ceb550b926d1c35eb38285086042f1856b018e2696d8b2a052291d2b40968efc09c3e395d9e1c9e34437a1502be6031437fe0ba2d100baa9"],"id":1}' -H "Content-Type: application/json" https://testnet.hashio.io/api

Returns response:

{
  "jsonrpc": "2.0",
  "id": 1,
  "error": {
    "code": 32001,
    "name": "Nonce too low",
    "message": "[Request ID: 84fb1193-0a3c-4732-b0e2-5b105f102553] Nonce too low. Provided nonce: 0, current nonce: 0"
  }
}
stoqnkpL commented 11 months ago

Okay, I know what happened.

  1. Gnosis signed a transaction, lets call it t, with signer, nonce and gasPrice - they used the current network gasPrice
  2. Between the time when they signed t and the time when you attempted to execute t, the network gasPrice grew, hence t is now invalid
  3. You sent t and it failed with INSUFFICIENT_TX_FEE
  4. On the services node, executing t increased the signer nonce, even though t reverted. That's normal.
  5. However, due to a bug in services, the nonce increment was not written in the record stream and the Mirror node was not updated --- it's not super important to understand the details, except to understand that the Mirror node believes that signer's nonce is nonce, but it actually is nonce + 1.
  6. When attempting to re-execute the same transaction, or a new one but one that has the same value for nonce, services returns an error WRONG_NONCE which is correct. This is observable in the Relay logs.
  7. Finally, the Relay attempts to handle the error gracefully and return a meaningful message here. Unfortunately, due to the Mirror node not being updated with the signer nonce increment, the message ends up confusing and wrong.

TLDR is the transaction has an invalid nonce. It should be nonce+1. The Relay will start returning more meaningful and correct error messages once services is updated with the fix for the bug I mentioned. Thank you @mshakeg for surfacing this issue. cc @Nana-EC @steven-sheehy

mshakeg commented 11 months ago

@stoqnkpL, thanks for the explanation. Just to confirm: despite the transaction with nonce 0 not being processed due to a low gasPrice, has the nonce for account 0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37 been incremented to 1, if so this is contrary to typical EVM behavior where nonces don't increment for unprocessed/dropped transactions(due to for example the tx gasPrice being too low)?

stoqnkpL commented 11 months ago

@mshakeg yes, that's correct. The thinking here is the following: every transaction that has reached consensus and is included in a block should increase the signer nonce. This is equivalent behavior with Ethereum. This transaction is actually processed since it reached consensus and is included in a block. Had it been outright rejected by a precheck on the node it's submitted to, then it would not have increased the signer nonce. There are several prechecks for EthereumTransactions like fileID validation, chainID validation, whether it covers the intrinsic gas cost, whether an attempt to transfer a negative amount is made, etc, but we don't have a precheck for gasPrice. Adding @tinker-michaelj and @Nana-EC to the conversation to consider adding such a precheck.

mshakeg commented 11 months ago

@stoqnkpL thanks for clarifying that. I really think a gasPrice precheck should be added for behaviour consistent with Ethereum. It would be even better if account 0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37 can have its nonce reset back to 0 so I can retry the transaction signed by the Gnosis Safe team and deploy a canonical version of their contracts on Hedera.

Nana-EC commented 11 months ago

@stoqnkpL thanks for clarifying that. I really think a gasPrice precheck should be added for behaviour consistent with Ethereum. It would be even better if account 0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37 can have its nonce reset back to 0 so I can retry the transaction signed by the Gnosis Safe team and deploy a canonical version of their contracts on Hedera.

@mshakeg as part of equivalence we're exploring the correct behavior noted. However, we can't breach the sovereignty of an account and reset its nonce. Are you able to submit a transaction with a correct nonce? This could be using a wallet or a web3 tool. This should unblock you whiles we explore resolving this issue for all accounts as part of equivalence.

mshakeg commented 11 months ago

@Nana-EC the Gnosis Safe team manages address 0xE1CB04A0fA36DdD16a06ea828007E35e1a3cBC37 and sign a tx with nonce 0 for the deployment of a canonical safe-singleton-factory across all chains.

I can deploy a non-canonical safe-singleton-factory by an account managed by me, but then Hedera won't have a canonical Gnosis Safe deployment.

shemnon commented 11 months ago

I think this is a bug in Hedera and a bug in our Ethereum Equivalence. If the transaction fails prior to entry into the EVM we should not increment the nonce, even if this happens at consensus.

I also think errata to reset the account nonce to remove the incorrect increment is not "breeching the sovereignty" of the account as it was an untyped transaction not targeting Hedera. The intent of the transaction was for it to work in EE ways across all chains.