Allow non-owners to execute confirmed (rejection) transactions

[iamacook]

What it solves

Resolves #1811

How this PR fixes it

Transactions with sufficient confirmations are now executable by non-owners.

The canExecute action flag was modified to not require isUserAnOwner. All instances of the isUserAnOwner were remove, where logical. Associated styling was updated and unnecessary props removed.

Note: a bug within processTransaction() was fixed by including a fallback for txHash when an error occurs.

How to test it

1. Create a transaction on a safe that has a threshold higher than 1.
2. Confirm said transaction in order to reach the treshold, but do not execute the transaction.
3. Open the transaction under an account that is not an owner of the Safe. It should be possible to execute the transaction without error.

The above is also possible with rejection transactions (see below).

Screenshots

Issues found:

• Can't sign with a 2nd owner: Allow non-owners to execute confirmed (rejection) transactions #3151 (comment)

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

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[github-actions]

Deployment links

🟠 Safe Rinkeby	⚪ Safe Mainnet	🟣 Safe Polygon	🟡 Safe BSC	⚫ Safe Arbitrum	🟢 Safe xDai

[coveralls]

Pull Request Test Coverage Report for Build 1582294975

• 2 of 11 (18.18%) changed or added relevant lines in 8 files are covered.
• 2 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.02%) to 32.231%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/components/TransactionFailText/index.tsx	0	1	0.0%
src/logic/safe/store/actions/processTransaction.ts	0	1	0.0%
src/logic/safe/utils/aboutToExecuteTx.ts	0	1	0.0%
src/routes/safe/components/Transactions/TxList/TxCollapsed.tsx	0	1	0.0%
src/routes/safe/components/Transactions/TxList/TxDetails.tsx	1	2	50.0%
src/routes/safe/components/Transactions/TxList/hooks/useActionButtonsHandlers.ts	0	1	0.0%
src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts	0	3	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/logic/safe/utils/aboutToExecuteTx.ts	1	8.0%
src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts	1	3.92%

Totals
Change from base Build 1577488613:	-0.02%
Covered Lines:	3088
Relevant Lines:	8502

---

💛 - Coveralls

[github-actions]

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

Failed tests:

• ❌ Address book Address book
• ❌ Read-only transaction creation and review Create and review a Send Funds transaction
• ❌ Safe Balances Safe Balances

[katspaugh]

Code looks good.
Could you check that the Execute button is still deactivated when you're on a wrong chain?
Also please don't merge for now, to keep dev available for RC fixes.

[katspaugh]

Nice! 👍

[DiogoSoaress]

Looks good!

[johannesmoormann]

Single owner safes work as well, but currently get this warning message.

[iamacook]

@johannesmoormann, can you give it another try after the deploy has finished?

[francovenica]

Safe:
https://pr3151--safereact.review-safe.gnosisdev.com/app/rin:0xcACCB4c94376eed9390c2d304fD54113a1fFC9e1/transactions/queue

Issues:
I can't sign with the 2nd owner of my safe. The sign button simply is not showing up
I just created a tx with a safe that requires 2 owners. the 2nd one wasn't even shown the sign button.

[francovenica]

Question: cc @johannesmoormann
Can somebody who is not even connected to the app sign the tx?
I've tried and it seems to fail on every step, but the button to execute is there and a non-connected user is technically a non-owner as well.
So:
Should we allow non-connected users be able to execute tx OR
Should remove the "execute button" for non-connected users and establish the concept of "non-owners" as: a connected user that is not owner of the safe

Currently a owner not connected can see the execute button, but the gas estimation doesn't work, the "nonce" value for the user's wallet (not the safe nonce) and the execution of tx themselves don't work

[johannesmoormann]

@francovenica somebody who is not connected should not be able to execute the transaction through the interface. current behavior is ok though. we might want to show a message to the user, that he needs to connect his wallet in order to carry out the execution in the future.

[francovenica]

@johannesmoormann Ok, Then I'll ask for the execute button to be removed if there nobody connected to the safe app. Thanks

[katspaugh]

The execute button should be disabled, not removed.

[francovenica]

Suggestion. Should we put some mark to the accounts that executed the tx but are not owners? Until today it was expected that only owner can sign, so is almost understood that everyone in the list of people who signed/executed the tx are all owners

[iamacook]

Should we put some mark to the accounts that executed the tx but are not owners?

Do we have a preferred way of approaching this? Or does a simple label modification suffice? @johannesmoormann

[johannesmoormann]

I would wait to see if there is a need for this. Until then, I think it is perfectly fine to leave it as it is, since the tools to explore the executor are already there.

[francovenica]

The 2nd owner can sign now, so that issue was fixed.
I was able to sign with a connected user just fine every time. Tried with ledger, trezor, WC connection

One more issue I found:

Safe: https://pr3151--safereact.review-safe.gnosisdev.com/app/rin:0xcACCB4c94376eed9390c2d304fD54113a1fFC9e1/transactions/history

Now I can see the "Confirm" check in the history tab. Go to any safe in the history tab and hover the a tx.

[francovenica]

The confirm button is no longer in the history tab. That's fixed

Something has changed. I can no longer sign with a non-owner. I get an error in the console stating that
In this safe there is a tx ready to be executed, so you can try with this one:
https://pr3151--safereact.review-safe.gnosisdev.com/app/rin:0xcACCB4c94376eed9390c2d304fD54113a1fFC9e1/transactions/queue

[Uxio0]

Insights from the meeting I've just had with @iamacook:

• There's no need to call the transaction service when only executing a Safe Transaction. It will be picked by the transaction service as soon as it's executed. Yesterday there were issues on Rinkeby and that's why your testing didn't work. You can try again now.
• You need to call the transaction service when a tx is signed, it doesn't matter if it's just sign or sign and execute.

Let me know if you have any questions 🙂

[francovenica]

Ok, the tx is going through just fine now when signed by non owners, but we still have that error in the console still popping. We can ignore it for this ticket, but I'll create a new one to see if we can make it not show up at all

[Uxio0]

Ok, the tx is going through just fine now when signed by non owners, but we still have that error in the console still popping. We can ignore it for this ticket, but I'll create a new one to see if we can make it not show up at all

Yes please. It's not good to have an error on the console and also to do not required requests to the service

[JagoFigueroa]

Hola guys, I see that the message for when trying to create & execute a tx from a safe-app with a non owner has changed from the previous: 'You are currently not an owner of this Safe and won't be able to submit this tx' that was notifying the user that an app cannot be used as a non owner.

The thing is that trying to use apps as a non owner I've found these 2 scenarios:

1. I try to create & execute a tx In a safe with 1 owner:

2. I try to create & execute a tx In a safe with 2 owner:

Edited to add a little bit more of context :)

[iamacook]

I see that the message for when trying to create & execute a tx from a safe-app with a non owner has changed

@JagoFigueroa, good catch! I've added an issue about it here.