Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Proper rejections #1652

Closed
tschubotz opened this issue Nov 25, 2020 · 5 comments · Fixed by #2096
Closed

Proper rejections #1652

tschubotz opened this issue Nov 25, 2020 · 5 comments · Fixed by #2096
Assignees
Labels
Feature 👑 New functionality

Comments

@tschubotz
Copy link
Member

tschubotz commented Nov 25, 2020

What is this feature about? (1 sentence)

This ensures that rejections can be properly created and confirmed with the migration to the uniform tx list view (#820 )

Why is it needed? What is the value? For whom do we build it?

  • Transactions with the same nonce are competing - only 1 can be executed.
  • In order to “reject” a transactions, another transaction needs to be executed with the same nonce, thereby overwriting it.
  • Cancel transactions are technically just empty transactions with the same nonce.
  • On mobile:
    • No way to reject a tx, currently.
    • In the tx list, rejections aren't marked as such, but they are empty contract interactions.
  • On web/desktop:
    • Cancel txs not marked explicitly

High-level overview of the feature

  • User can reject transactions on mobile.

  • "rejecting" means creating a cancel tx. We decided to be explicit about what's happening under the hood.

  • Users can confirm such cancel tx.

  • Users can open / view a cancel tx.

  • Web and mobile should behave similar.

  • Change wording (Cf. Slack discussion)

    • awaiting confirmations -> needs confirmations
    • awaiting your confirmation -> needs your confirmation
    • awaiting execution -> needs execution
  • Change "Cancelling transaction" to "On-chain rejection" (see designs)

  • Change the button "Cancel" to "Reject"

For a rejections, all params should be set to 0x/null/0 except to, i.e. including safeTxGas (Has been split out to separate ticket #1978)

Regular tx flow should be used for rejections, i.e. not the red popups anymore (cf. this comment below). No need to have them separate anymore.
Same like with regular txs, it should be possible to do off-chain rejections no matter if the threshold has been reach or not. Same like we have this "checkmark" to execute a tx or to just approve a tx as the last signer.

Misc

@tschubotz tschubotz added the Feature 👑 New functionality label Nov 25, 2020
@tschubotz tschubotz changed the title Proper rejections & adapt tx details Proper rejections Jan 20, 2021
@tschubotz
Copy link
Member Author

@posthnikova is there a screen for executing a rejection on Zeplin already? Basically this one. Somehow I can't find it.

And just to double check: The latest designs on the rejection flow is this, right?

@posthnikova
Copy link

@tschubotz I was thinking a regular Execute popup will we used here (https://zpl.io/VDXKYyJ), because:
• After the rejection has been created the timeline in details looks like a regular timeline.
• Everything is green, Execute button is green so a red button in the popup would not fit into this flow.

image

And just to double check: The latest designs on the rejection flow is this, right?

Yes.

@tschubotz
Copy link
Member Author

@tschubotz I was thinking a regular Execute popup will we used here (https://zpl.io/VDXKYyJ), because:
• After the rejection has been created the timeline in details looks like a regular timeline.
• Everything is green, Execute button is green so a red button in the popup would not fit into this flow.

What you are proposing makes much more sense. Thanks for clarifying!

@nicosampler
Copy link
Contributor

I took a look at this ticket, I think it's pretty clear what we need to do.

But there is a technical issue that I think we should tackle first. In the code, we are replicating practically the same code for all the modals, it was kinda difficult to implement tx-parameters because we needed to track modal by modal and apply the same logic, and something similar will happen with this ticket and also for this one #898. So I suggest first creating a ticket in order to refactorize/unify all the modals.

@dasanra wdyt?

@fernandomg
Copy link
Contributor

fernandomg commented Mar 26, 2021

@lukasschor, I'm nitpicking here but, shouldn't we update the "cancel" concept in favor of "reject" in the article too? https://help.gnosis-safe.io/en/articles/4738501-why-do-i-need-to-pay-for-cancelling-a-transaction

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature 👑 New functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants