Fix: propose when creating or signing

[katspaugh]

What it solves

Resolves #3448

How this PR fixes it

In 3.17.2, before we refactored create/processTransaction, it worked fine.

Process tx:
https://github.com/gnosis/safe-react/blob/release/3.17.2/src/logic/safe/store/actions/processTransaction.ts#L166-L168
https://github.com/gnosis/safe-react/blob/release/3.17.2/src/logic/safe/store/actions/processTransaction.ts#L135-L138

Create tx:
https://github.com/gnosis/safe-react/blob/release/3.17.2/src/logic/safe/store/actions/createTransaction.ts#L167-L169
https://github.com/gnosis/safe-react/blob/release/3.17.2/src/logic/safe/store/actions/createTransaction.ts#L197-L198

Basically, it proposes when signing, or when creating a new tx.

This PR restores that logic.

[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}

[github-actions]

Deployment links

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

[iamacook]

As this gets WC approvals 'working', I'll approve this. However, we are over-proposing and errors will appear in the console when non-owners execute.

[coveralls]

Pull Request Test Coverage Report for Build 1817905835

• 1 of 12 (8.33%) changed or added relevant lines in 4 files are covered.
• 21 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+4.0e**-05%**) to 33.503%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/logic/safe/store/actions/processTransaction.ts	0	1	0.0%
src/logic/safe/store/actions/createTransaction.ts	0	2	0.0%
src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx	0	8	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/logic/safe/store/actions/createTransaction.ts	1	3.01%
src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx	1	3.42%
src/components/AppLayout/Sidebar/SafeHeader/index.tsx	19	0%

Totals
Change from base Build 1813771916:	4.0e-05%
Covered Lines:	3210
Relevant Lines:	8547

---

💛 - Coveralls

[github-actions]

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

Failed tests:

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

[katspaugh]

While testing this, I've encountered some other strange bugs with WC + Safe as a signer.

Need to look more closely into it.

[iamacook]

While testing this, I've encountered some other strange bugs with WC + Safe as a signer.

I will revisit the original refactor of these two functions on Friday.

[katspaugh]

What I found out about WC+Safe as a signer:

• it can create and "sign" transactions, but the signature is empty
• we can still propose these txns to the backend, and it will navigate to the proposed tx
• it will not register a confirmation (because the sig is empty)

The same behavior occurs on 3.17.2, and on this branch (after fixing the proposal conditions).

I've also refactored processTransaction a bit more:

• Removed the useless shouldExecuteTransactionCondition because it was always returning false. It only makes sense in createTransaction, not in process.
• Changed the logic around preApprovingOwner so that we only send it when only 1 confirmation is missing and send is an owner

[iamacook]

There are a lot of cases to check but I trust your logic. I have a few questions, nonetheless

[katspaugh]

I've tested the following cases:

With MM:

• Creating a tx w/o immediate execution in a 1/1 Safe, executing later as owner
• Creating a tx w/o immediate execution in a 1/1 Safe, executing later as a non-owner
• Creating a tx w/o immediate execution in a 2/3 Safe, second owner confirming and executing
• Creating a tx w/o immediate execution in a 2/3 Safe, second owner confirming, a non-owner executing

With WC and the Safe mobile app as signer (imported EoA keys):

• Creating a tx
• Signing a tx
• Executing – doesn't work, the same was on 3.17.2 (I think this has never been supported?)

With WC and a Safe as signer:

• Creating a tx – proposes but w/o the initial signature (it's empty), shows up in the queue
• Confirming/executing – signature is empty, so essentially doesn't work (same as on 3.17.2)

[iamacook]

If this mirrors the behaviour from 3.17.2 then we can assume that the regression has been tackled?

Thank you for the breakdown. I will look into this deeper on Friday.

[katspaugh]

Yes, basically the fix is that it now proposes a tx on creation, albeit without a signature.

[francovenica]

I tested regular tx in safes 1/1 and 2/2 and I had no issues creating, executing, rejecting tx with owners and non-owners

I have this safe where the owners are a MM account and another safe
I connect the safe via WC
I was able to create tx with the safe as the owner. The tx is created with 0 signatures at first, but after the tx is successfully executed in the owner safe it changes on its own to have 1 signature already set, so that's working fine
I was able to just confirm tx and the confirm and execute all in the same action

The safe I was testing on
https://pr3455--safereact.review-safe.gnosisdev.com/app/rin:0x978b1681be5e741D888cB8DE076011DE11dE8072/transactions/history

The safe owner rin:0xd7F4a1Cca3428eC09bC0fC9dBDFcDb087803fD59

So far it looks good. The only thing I'm missing to test is with the mobile app.

[katspaugh]

It's fine, I've tested with the mobile app, it works as good as MM.
Thanks Franco!