fix: do not show nonce conflict warning for batch transactions.

[jmealy]

What it solves

Resolves #3502

How this PR fixes it

Displays batch transactions as a single transaction instead of a group of transactions.

How to test it

• Create a batch tx and sign
• Create any other tx and sign
• bulk execute the two Txs
• There should be no warning about conflicting nonces within the bulk tx, as they are all executed as one tx with the same nonce

Screenshots

Checklist

• [ ] I've tested the branch on mobile 📱
• [ ] I've documented how it affects the analytics (if at all) 📊
• [ ] I've written a unit/e2e test for it (if applicable) 🧑‍💻

[github-actions[bot]]

Branch preview

✅ Deploy successful!

Website: https://fix_batch_warning--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook: https://fix_batch_warning--walletweb.review.5afe.dev/storybook/

[github-actions[bot]]

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	0	N/A

• Result: :white_check_mark: success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[github-actions[bot]]

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

[github-actions[bot]]

Coverage report

St.:grey_question:	Category	Percentage	Covered / Total
🟡	Statements	79.26%	11630/14673
🔴	Branches	58.15%	2753/4734
🟡	Functions	66.33%	1866/2813
🟢	Lines	80.62%	10481/13001

Test suite run success

1468 tests passing in 202 suites.

Report generated by 🧪jest coverage report action from f0f7b8018c94227679a3f5868d386fe5fdd15fdf

[usame-algan]

I didn't understand how this can happen so I looked a bit more into it. From what I can see this only happens when using the Batch execute feature in a 1/n safe:

1. Whenever a user presses "Add to Batch", an unsigned transaction is proposed to the backend
2. When eventually confirming the batch, a multisend is created, signed and proposed to the backend
3. All of these transactions have the same nonce so there is a conflict header
4. When the user executes a transaction we fetch the untrusted queue which contains the previously mentioned transactions. We filter all of them out except the one(s) that are pending.

If the user were to execute this multisend on its own it would work because we have logic in place inside usePendingTxs to remove the conflict header item if only one transaction is pending. Not sure if this is intended or a "feature".

However if they queue another transaction and use the "Bulk execute" feature there is more than one pending transaction so the conflict header is not removed but we remove all transactions that are not pending so we end up with only one transaction that has a conflict header. Inside groupConflictingTxs we detect that and then push that transaction into an array which causes this display issue.

Imo the logic inside TxList and groupConflictingTxs is correct because we assume that if there is a conflict header that there are at least 2 transactions conflicting. Instead we should look into usePendingTxs and why we are removing the conflict header in some cases but not others and why we are only looking at the first two items there.

For example if I change this block it also resolves the issue (but might break other things)

if (results[1] && isConflictHeaderListItem(results[1])) {
  // Check if we both conflicting txs are still pending
  if (results.filter((item) => isTransactionListItem(item)).length <= 1) {
    results.splice(1, 1)
  }
}

to

if (results[1] && isConflictHeaderListItem(results[1])) {
    results.splice(1, 1)
}

[github-actions[bot]]

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	0	N/A

• Result: :white_check_mark: success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[jmealy]

I didn't understand how this can happen so I looked a bit more into it. From what I can see this only happens when using the Batch execute feature in a 1/n safe:

1. Whenever a user presses "Add to Batch", an unsigned transaction is proposed to the backend
2. When eventually confirming the batch, a multisend is created, signed and proposed to the backend
3. All of these transactions have the same nonce so there is a conflict header
4. When the user executes a transaction we fetch the untrusted queue which contains the previously mentioned transactions. We filter all of them out except the one(s) that are pending.

If the user were to execute this multisend on its own it would work because we have logic in place inside usePendingTxs to remove the conflict header item if only one transaction is pending. Not sure if this is intended or a "feature".

However if they queue another transaction and use the "Bulk execute" feature there is more than one pending transaction so the conflict header is not removed but we remove all transactions that are not pending so we end up with only one transaction that has a conflict header. Inside groupConflictingTxs we detect that and then push that transaction into an array which causes this display issue.

Imo the logic inside TxList and groupConflictingTxs is correct because we assume that if there is a conflict header that there are at least 2 transactions conflicting. Instead we should look into usePendingTxs and why we are removing the conflict header in some cases but not others and why we are only looking at the first two items there.

For example if I change this block it also resolves the issue (but might break other things)

if (results[1] && isConflictHeaderListItem(results[1])) {
  // Check if we both conflicting txs are still pending
  if (results.filter((item) => isTransactionListItem(item)).length <= 1) {
    results.splice(1, 1)
  }
}

to

if (results[1] && isConflictHeaderListItem(results[1])) {
    results.splice(1, 1)
}

@usame-algan Thanks, I didn't see this header removal part you pointed out. After looking into it again I can't see any reason for why we are assuming the conflict header is always in the same position, so I'm assuming it's just an oversight. ie. it is not taking into account when there are single transactions preceding the batch, or when there are more than one batches. I made the conflict header removal section you pointed out more general to handle these cases where the header doesn't have index 1 in the array or where there is more than one of them.

This seems to work and I haven't found anything so far that it breaks, but it seems like it would be better if these cases were accounted for at the point where these headers are added (in the CGW I'm assuming).

[francovenica]

@jmealy The environment is showing a 404 when you access it

[github-actions[bot]]

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	0	N/A

• Result: :white_check_mark: success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[francovenica]

So I currently have a problem that won't allow me to fully test this ticket

Right now executing a tx in bulk is not exiting the form (I read about this in another ticket or a slack conversation). Also the tx in queue are not showing the "pending" status that usually shows when tx are being executed, so the scenario where this was failing before is not met, therefore is not possible to test the fix.

I'm not saying that the fix is not correct or there is something wrong with it, just saying that, given how tx in bulk is working here, is not possible to test the fix

I'll keep this ticket here for the time

[usame-algan]

Right now executing a tx in bulk is not exiting the form (I read about this in another ticket or a slack conversation). Also the tx in queue are not showing the "pending" status that usually shows when tx are being executed, so the scenario where this was failing before is not met, therefore is not possible to test the fix.

This is an issue with MetaMask that we track here https://github.com/safe-global/safe-wallet-web/issues/3675. Could you test this ticket with a different wallet like Rabby instead? It should work fine there.

[francovenica]

True, I tried with Rabby and I didn't have that issue so I was able to test the fix

LGMT. The "Conflicting nonce" message doesn't show anymore for tx with multiple actions in it.

[github-actions[bot]]

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	0	N/A

• Result: :white_check_mark: success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}