MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.92k stars 4.88k forks source link

Batching Issues #5817

Closed willfsays closed 4 years ago

willfsays commented 5 years ago

Describe the bug When batching a request to approve (with an ERC 20) and then to send tokens, MetaMask reverses the request, you have to add the send to the batch first before the approve to make it work.

When firing the same two async requests without a batch, as per above, MetaMask reverses the request, you have to add the send to the batch first before the approve to make it work.

To Reproduce Steps to reproduce the behavior:

  1. Create a standards-based ERC20 with approve and allowance methods
  2. Create another method that checks the allowance >= an amount passed into the function (with a require) (lets call this setValue)
  3. Create a new web3.BatchRequest() (web3 version 1.0.0.36)
  4. Add batch send to ERC20 approve (e.g. contract.methods.approve(address, 1000).send({from: accountAddress});)
  5. Add batch send to ERC20 setValue (e.g. contract.methods.setValue(1000).send({from: accountAddress});)
  6. run batch.execute()
  7. MetaMask will batch the requests in the wrong order (5 before 4) - proven by the nonce allocated
  8. If you reverse the batch order, they run in the expected order (although exception is still seen for setValue even after confirming approve)
  9. Create a standards-based ERC20 with approve and allowance methods
  10. Create another method that checks the allowance >= an amount passed into the function (with a require) (lets call this setValue)
  11. call send method to approve (e.g. contract.methods.approve(address, 1000).send({from: accountAddress});)
  12. call send method to setValue (e.g. contract.methods.setValue(1000).send({from: accountAddress});)
  13. MetaMask will batch the requests in the wrong order (4 before 3) - proven by the nonce allocated
  14. If you reverse the call order, they run in the expected order (although exception is still seen for setValue even after confirming approve)

Expected behavior MetaMask will batch the requests in the order in which they are received

Browser details (please complete the following information):

adamsoffer commented 4 years ago

@willfsays Confirmed on my end as well. BatchRequest does not work with MetaMask. Metamask batches the requests in the incorrect order. I've tested with a couple other web3 providers (fortmatic and portis) and they both honor the batch request order. To reproduce, try batching 3 or more send transactions. MetaMask will ask you to confirm the transactions in the following order: 1, 3, 2

cc: @danfinlay

danfinlay commented 4 years ago

Closing in favor of #5852.