Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reference transaction trusted flag when showing the UnsignedWarning #1220

Closed
iamacook opened this issue Nov 22, 2022 · 6 comments · Fixed by #1226
Closed

Reference transaction trusted flag when showing the UnsignedWarning #1220

iamacook opened this issue Nov 22, 2022 · 6 comments · Fixed by #1226

Comments

@iamacook
Copy link
Member

iamacook commented Nov 22, 2022

What is the feature about

The UnsignedWarning is currently shown when no signatures are present. However, the gateway will soon forward the trusted flag of each transaction as well, which should also be referenced.

The list of requirements

The conditional rendering logic needs to be extend to include the trusted flag as follows:

{!trusted && <UnsignedWarning />}

Links

@katspaugh
Copy link
Member

Shouldn't we use only the trusted flag?
@Uxio0 it's always trusted=true for signed transactions, right?

@Uxio0
Copy link
Member

Uxio0 commented Nov 23, 2022

Yes, trusted flag is all you need. Transactions signed by at least one owner are trusted. Even if you remove the owner, transaction will still be trusted

@iamacook iamacook self-assigned this Nov 23, 2022
@iamacook iamacook removed the blocked label Nov 23, 2022
@iamacook iamacook linked a pull request Nov 23, 2022 that will close this issue
@katspaugh katspaugh reopened this Nov 25, 2022
@katspaugh
Copy link
Member

katspaugh commented Nov 25, 2022

@francovenica the trusted flag is now on the staging backend, please test the following cases:

The fix is on dev (https://safe-web-core.dev.5afe.dev/).

  • A delegate-created tx should NOT be marked as untrusted
  • MMI txns should NOT be untrusted
  • Regular transactions proposed w/o a signature SHOULD be untrusted – you can create them on this branch https://drafts.web-core.pages.dev – just uncheck the Sign checkbox in the tx modal. But view them via a deep link on dev.

@iamacook
Copy link
Member Author

* Regular transactions proposed w/o a signature SHOULD be untrusted – you can create them on this branch https://drafts.web-core.pages.dev – just uncheck the Sign checkbox in the tx modal. But view them via a deep link on dev.

I just created an unsigned transaction on staging here for you to confirm: gor:0x65c57CC1317a1f728E921Cbf7bC08b3894196D29/transactions/tx?id=multisig_0x65c57CC1317a1f728E921Cbf7bC08b3894196D29_0xec49a9f9eee0446834c1c4ee00ca1b366f604d5e09555c34c9cd9a29c508757f

image

@JagoFigueroa
Copy link

Adding this scenario affecting safe apps:

untrusted.mp4

When the pop up to view the transaction is clicked, txs are showing as untrusted for me until the transaction is indexed. Then it will lose this status.

@katspaugh
Copy link
Member

@JagoFigueroa fixed in #1261. Immediately executed transactions in 1/x Safes won't be visible until executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants