Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BatchRequest is not working properly with metamask #6796

Closed
rstormsf opened this issue Jul 3, 2019 · 7 comments
Closed

BatchRequest is not working properly with metamask #6796

rstormsf opened this issue Jul 3, 2019 · 7 comments

Comments

@rstormsf
Copy link

rstormsf commented Jul 3, 2019

https://codepen.io/rstormsf/pen/bPKxBv?editors=1010
Steps:

function metamaskBug() {
  const web3_beta34 = new Web3(window.web3.currentProvider)
  window.ethereum.enable().then(async (res) => {
    const acc = res[0]
    const batch = new web3_beta34.BatchRequest()

    const tx = web3.eth.sendTransaction.request(
      {
        from: acc,
        to: '0x0039F22efB07A647557C7C5d17854CFD6D489eF3',
        gas: '0x186A0',
        gasPrice: '0x3B9ACA00',
        value: '0x0',
        data: '0x4869207468657265'
      },
      () => {}
    )
    batch.add(tx)
    const tx2 = web3.eth.sendTransaction.request(
      {
        from: acc,
        to: '0x0039F22efB07A647557C7C5d17854CFD6D489eF3',
        gas: '0x186A0',
        gasPrice: '0x3B9ACA00',
        value: '0x0',
        data: '0x4869207468657265'
      },
      () => {}
    )
    batch.add(tx2)
    const { response } = await batch.execute()
    })
}

Notice how the UI responds to the second transaction.
metamaskBug2
Demo: https://codepen.io/rstormsf/pen/bPKxBv?editors=1010

Issues:
The UI doesn't display passed in values: gas, gasLimit, etc even though it actually knows them since the tx goes thru if I submit them

@danfinlay
Copy link
Contributor

Was this a sudden change you noticed in behavior, or is this a longer lasting issue?

Also please check your version: We just deployed 6.7.2, which fixed a similar issue: #6777, #6772

@rstormsf
Copy link
Author

rstormsf commented Jul 4, 2019

@danfinlay
Now i see a different behavior on 6.7.2
On 1st tx, it shows as send Ether
image

on 2nd tx, it shows as contract interaction
image

@danfinlay
Copy link
Contributor

We recently added some new methods of detecting method signatures, and it seems your method signature 0x48692074 is being detected as SENT ETHER (not to be confused with SEND ETHER). This lookup has a timeout, and so at first glance, it seems like your second window rendered before the response could get back in time.

I'm not sure where that sent ether key is coming from yet, as it's neither on 4byte nor the parity on-chain method registry, we'll have to look into that, but otherwise this looks approximately correct.

Another issue I notice is that your recipient address is short, and does not resemble an Ethereum address, which is very strange and we'll have to also check for causes of. Looks like maybe the ellipses is getting cut off on your screen for some reason.

@whymarrh
Copy link
Contributor

whymarrh commented Jul 4, 2019

@rstormsf thanks for the pen, that's a huge help! I see the method name for both transactions when tabbing through the txs, do you see the same on your end?

I want to confirm that this behaves the same for both of us. Similar to @danfinlay, I find the rendering of the recipient address strange, as it seems to render fine on my end. 😬

I can reproduce the incorrect method names, as you outlined, when I confirm the first tx:

Thanks again for the pen eh, that's extremely helpful.

@rstormsf
Copy link
Author

rstormsf commented Jul 4, 2019

@danfinlay
as the data, I used

web3.fromAscii('Hi there')
"0x4869207468657265"

just to say hi to you ;-)

@danfinlay my address has a name 0039 is just the tag that I personally use (named address feature in metamask instead of Account 1)
image

@whymarrh My 5 yrs experience as professional QA engineer pays off ;-) Can't file bugs without real proof

@rstormsf
Copy link
Author

rstormsf commented Jul 4, 2019

feel free to check this usage on real Dapp: https://multisender.app on testnets(esp Kovan), you will see that for some reason 1st tx always thinks it's send ether, not contract interaction

@tmashuang
Copy link
Contributor

Closing this as fixed. Tested both links of reproduction, codepan and multisender, and transactions on both sites successfully went through in the correct order and parameters. If this is not the case, please comment and I'll reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants