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

Tx marked as "This transaction will most likely fail" even though it doesn't #1804

Closed
tschubotz opened this issue Jan 22, 2021 · 3 comments · Fixed by #1808
Closed

Tx marked as "This transaction will most likely fail" even though it doesn't #1804

tschubotz opened this issue Jan 22, 2021 · 3 comments · Fixed by #1808
Assignees
Labels
Bug 🐛 Something isn't working Major Needs to be fixed for immediate next public release.

Comments

@tschubotz
Copy link
Member

tschubotz commented Jan 22, 2021

This is based on a report from an external team on Slack, however I reproduced this on Rinekby.

Title/Description

Tx marked as "This transaction will most likely fail" even though it doesn't

Environment

Steps to reproduce

Prerequisites:

Now the actual steps:

  1. Open contract interaction.
  2. use the erc721 token address from above (0xAd9104d92DB0215A4397511f06E79C01bA757c1a)
  3. select transferFrom
  4. Enter id=7 to from=0x00984CBdcAc32676d811f5bDB82bC50ae6dd6784 and to=0x0e329fa8d6Fcd1ba0Cda495431F1f7CA24F442C2
  5. Review and create tx with owner 1
  6. Now with owner 2, try to confirm the tx

Expected result

No warning

Obtained result

Warning shows up.

Misc

Screenshots

image

@tschubotz tschubotz added Bug 🐛 Something isn't working Major Needs to be fixed for immediate next public release. labels Jan 22, 2021
@tschubotz
Copy link
Member Author

closing as it's not reproducible. I actually did execute it before already, hence the error above is expected.

@tschubotz
Copy link
Member Author

Alright, now reliably reproducible, I updated the steps above. Reopening.

@tschubotz tschubotz reopened this Jan 22, 2021
@tschubotz
Copy link
Member Author

I had the same issue with sending testnet DAI with my Safe: https://rinkeby.gnosis-safe.io/app/#/safes/0xaE3c91c89153DEaC332Ab7BBd167164978638c30/transactions just now.

dasanra pushed a commit that referenced this issue Jan 26, 2021
* Fix checkIfTxIsExecution method implementation

* Add tests for checkIfTxIsExecution/checkIfTxIsCreation/checkIfTxIsApproveAndExecution/

* Minimice number of ifs with same result

Co-authored-by: Mati Dastugue <[email protected]>
Co-authored-by: Daniel Sanchez <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug 🐛 Something isn't working Major Needs to be fixed for immediate next public release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants