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

fix: transaction stage-relative execution #3323

Merged
merged 1 commit into from
Jan 19, 2022
Merged

Conversation

iamacook
Copy link
Member

What it solves

Resolves #3305

How this PR fixes it

Instead of blanket establishing isExecution within TxSender, the logic is now extracted into the respective transaction creation/processing function as the previous logic was before the refactor.

On top of this, the transaction proposal to backend now depends on whether the user is an owner.

How to test it

Every variation of transaction is affected by this. The cases are as follows:

1/? threshold:

  1. Create and execute transaction immediately.
  2. Queue transaction and execute later by owner.
  3. Queue transaction and execute by non-owner.

2+/? threshold:

  1. Create and confirm transaction then execute by owner(s).
  2. Create and confirm transaction then execute by non-owner.

Safe creation

  1. Create Safe successfully.

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

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

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1714003035

  • 0 of 5 (0.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 32.441%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/logic/safe/store/actions/processTransaction.ts 0 2 0.0%
src/logic/safe/store/actions/createTransaction.ts 0 3 0.0%
Totals Coverage Status
Change from base Build 1713602320: -0.01%
Covered Lines: 3163
Relevant Lines: 8701

💛 - Coveralls

@github-actions
Copy link

Deployment links

🟠 Rinkeby Mainnet 🟣 Polygon 🟡 BSC Arbitrum 🟢 Gnosis Chain

@github-actions
Copy link

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

Failed tests:

  • ❌ Safe Apps List Safe Apps List

@francovenica
Copy link
Contributor

Looks good to me.

Also checked tx rejections and tried with my trezor and ledger as well

@iamacook
Copy link
Member Author

iamacook commented Jan 19, 2022

I'm merging this to fix the error on dev as it has been tested and is working. However, we are still proposing when we shouldn't be. This doesn't cause issues (and was this way for months) but we should still try to fix it when we get the chance. I've created an issue for that here.

@iamacook iamacook merged commit c85ae7f into dev Jan 19, 2022
@iamacook iamacook deleted the tx-execution-refactor branch January 19, 2022 11:18
@github-actions github-actions bot locked and limited conversation to collaborators Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tx ready for execution are popping up the wrong type of tx signature
4 participants