Fix: use recommended nonce in the nonce warning

[katspaugh]

What it solves

Resolves #3506

How this PR fixes it

The condition to show a nonce warning was relying on lastTxNonce, whereas the warning itself was displaying a recommendedNonce.

Now it's using recommendedNonce in both places, so it should never show the absurd message from the bug report.

I've also renamed the useRecommendedNonce hook.

How to test it

• Try to create transactions with a nonce out of order.
• Try a nonce in future
• Try a nonce in the past

[github-actions]

CLA Assistant Lite All Contributors have signed the CLA.

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	1	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[coveralls]

Pull Request Test Coverage Report for Build 1877151044

• 12 of 13 (92.31%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.1%) to 34.684%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/routes/safe/components/Transactions/helpers/EditTxParametersForm/index.tsx	0	1	0.0%

Totals
Change from base Build 1877126792:	0.1%
Covered Lines:	3326
Relevant Lines:	8609

---

💛 - Coveralls

[iamacook]

There's a failing test that needs looking at.

[github-actions]

Deployment links

🟠 Rinkeby	⚪ Mainnet	🟣 Polygon	🟡 BSC	⚫ Arbitrum	🟢 Gnosis Chain

[github-actions]

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1882338094

Failed tests:

• ❌ Read-only transaction creation and review Read-only transaction creation and review
• ❌ Safe Balances Safe Balances

[DiogoSoaress]

The fix makes total sense. 👍
There is a failing test however.

[usame-algan]

Looking good!

[usame-algan]

🚀

[francovenica]

Issue:
The warning is popping up even if a tx was not executed.
The warning should be there if the nonce that is tried to be used is from an already executed tx.

In this snapshots I've created the tx 141, which cannot be executed because the tx 140 needs to be executed first.
Also when I try to create the 140 (which at this point doesn't exist yet) the warning tells me that it has already been used.

[katspaugh]

In this snapshots I've created the tx 141, which cannot be executed because the tx 140 needs to be executed first.

I think the warning in this case still makes sense. You can only execute them in order, so 140 -> 141 -> 142.
Transactions 140 and 141, although they haven't been executed yet, have already been proposed to the backend. So they do exist, and backend recommends to use the next nonce correctly.

[francovenica]

In this snapshots I've created the tx 141, which cannot be executed because the tx 140 needs to be executed first.

I think the warning in this case still makes sense. You can only execute them in order, so 140 -> 141 -> 142. Transactions 140 and 141, although they haven't been executed yet, have already been proposed to the backend. So they do exist, and backend recommends to use the next nonce correctly.

No, tx nonce 140 doesn't exist at all, it was never created, so it's not in the backend in this case

[katspaugh]

Oh, now I get it. You skipped 140 and created 141 by overriding the nonce of the previous tx.

Still, the backend returns 142 as the recommended nonce, and the warning message relies on that.

[katspaugh]

@francovenica does this fix solve the issue described in #3506 though?

[katspaugh]

I've rephrased the warning a bit, so that it doesn't assert that nonce has been used necessarily.

[francovenica]

We still have issues. This is the attempt of execution of the current tx. It seems that, because the recommended nonce (the one that will be suggested when I create a new tx) is 1 more, the warning message is shown.
I think the message should not be shown at all if the nonce is not editable (a tx already created or a tx that is about to be executed)

[katspaugh]

Great catch, @francovenica!
I'll adjust it to not show the warning in existing transactions unless the nonce was edited.

[francovenica]

The message looks fine, and the message not popping up if the nonce is non-editable is fine too.
I think we can merge it

Still, we should discuss further the issue of the warning message telling users that creating the tx that can actually be executed is not recommendable:
In this image tx 94 is the next nonce to be executed in this safe, but tx 95 already exist, ergo the warning message