AragonBlack / fundraising

Fundraising apps suite for Aragon organizations
https://fundraising.aragon.black/
GNU Affero General Public License v3.0
75 stars 43 forks source link

ALERT: Transaction Error. Exception thrown in contract code. #55

Closed john-light closed 5 years ago

john-light commented 5 years ago

When I click New Order -> Buy and enter 5 DAI then the following happens:

  1. MetaMask pops up an approval tx. I sign it.
  2. Immediately a second MetaMask box pops up with the following error shown: ALERT: Transaction Error. Exception thrown in contract code.

This error sounds scary. Nonetheless, my order did seem to go through.

osarrouy commented 5 years ago

Yes I've seen this one too. Strangely it didn't occur on our local ganache chains ...

I'm gonna investigate further but my take is that the approve transaction has not been mined yet when initiating the openBuyOrder one thus leading Metamask to believe the transaction will fail ...

Once more this approval pre-hook is handled at the client level so gonna check with Brett.

sohkai commented 5 years ago

This isn't something we can solve; this is Metamask trying to be too smart for its own good. There's really no way to work around this, we either:

We prefer the first so all the user action can happen at once.

john-light commented 5 years ago

@sohkai fwiw as a user I personally prefer the second option. It reduces confusion for me about this error message, in what order I should be signing the transactions, what if the second one is confirmed before the first one, etc.

sohkai commented 5 years ago

@john-light The problem with the second one is that on mainnet, your transaction might take ages to mine, and you'd probably prefer the pending transaction in the tx pool already rather than having to wait on the frontend.

We can perhaps make this experience better in the signing panel, where we could wait by default but still allow a user to immediately sign and send the second transaction if they like (knowing that their web3 provider might prompt them with a warning).

osarrouy commented 5 years ago

@sohkai @john-light I do feel that the actual solution is the less of a harm ... If we wait for the first transaction to be mined before we send the second and the user closes it browser tab then the whole chaining of transaction we be messed up.

This is a problem especially for approve pre-hooks since some ERC20s will revert it you try to re-approve a token whose actual allowance is not zero ...

@john-light Is it ok to close this issue for now since it's not really fundraising related ? Or maybe we wanna copy-paste in aragon/aragon before to keep track of the conversation ?

john-light commented 5 years ago

if it is not a fundraising specific error then it makes sense to me to close it