UI displaying wrong transaction nonce when quickly executing transactions

[johannesmoormann]

Bug description

When executing a transaction (tested with a 1/n Safe) and quickly creating a new transaction, the UI will show the nonce of the previously executed transaction but submit the transaction with the correct nonce (or at least a nonce higher then is being displayed).

Environment

• Browser: Chrome
• Wallet: MetaMask
• Chain: Ethereum mainnet and Görli

Steps to reproduce

With a 1/n Safe create and execute multiple transactions and observe the nonce being displayed in the UI.

Expected result

The submitted and signed nonce should always match the displayed nonce in the UI.

Obtained result

see video

https://user-images.githubusercontent.com/93598450/218501568-b099bce8-6b67-48bf-8921-343a3563bebf.mp4

Screenshots

[katspaugh]

I’ve reproduced the bug with the nonce magically incrementing after creating and immediately executing in a 1/X Safe.

I've created and executed this tx which showed nonce 18

But it landed in the history with nonce 19.

Why?

tl;dr: it’s because such txs don’t land in the queue and thus don’t affect the nonce counter on the backend.

Longer explanation

The contract doesn’t care what nonce a tx has in the data [edit]for unsigned transactions[/edit], it just increments its own nonce on each execution.

The nonce that we use in the UI is generated by the backend based on the queue.
However, if you create and execute a tx w/o signing it (the so-called "immediate execution"), it won't be added to the queue, because it’s “untrusted”. Consequently, it also won't increment the "recommended" nonce on the backend.
Only after the tx is actually executed and indexed, will the backend pick it up, and sync the nonce with the contract nonce.

Solution

IMHO the cleanest solution would be to make users sign all transactions on creation. Or at least when there are pending transactions.
Execution w/o signature is a cool gimmick, which we can leave for already signed transactions, but not for newly created ones.
This will solve this bug and also avoid untrusted txs in principle.

[mmv08]

The contract doesn’t care what nonce a tx has in the data, it always just increments its own nonce on each execution.

This is only true for pre-validated signatures (aka msg.sender based ones). For all other types nonce is a part of the signed transaction hash and a different nonce on-chain would result in a failed transaction authorization.

More context:
safe-global/safe-smart-account#187

[katspaugh]

Thanks for the correction, makes sense. 👍