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

V5 Beta, Overrides for Contract transactions not working #845

Closed
crazyrabbitLTC opened this issue May 22, 2020 · 5 comments
Closed

V5 Beta, Overrides for Contract transactions not working #845

crazyrabbitLTC opened this issue May 22, 2020 · 5 comments
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@crazyrabbitLTC
Copy link

crazyrabbitLTC commented May 22, 2020

I'm using the Ethersjs ^5.0.0-beta.188, and wondering if this is something I'm doing wrong, or what. The code worked previously when using V4, so not sure what I've done differently.

...
  const overrides = {
    gasLimit: estimatedGas,
    gasPrice,
    value: ethers.utils.parseEther("0"),
  }

try {
 const methodResult = await method(...methodParams, overrides)
} catch...

Gives the following error:

{
  "reason": "contract method is not payable",
  "code": "INVALID_ARGUMENT",
  "argument": "sendTransaction:shouldNotRevert(string)",
  "value": {
    "gasLimit": {
      "_hex": "0xa906",
      "_isBigNumber": true
    },
    "gasPrice": {
      "_hex": "0x3b9aca00",
      "_isBigNumber": true
    },
    "value": {
      "_hex": "0x00",
      "_isBigNumber": true
    },
    "to": {},
    "data": "0xd87f6dac00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000004706c706c00000000000000000000000000000000000000000000000000000000"
  }
}

The argument I am passing is is a normal string. (It is not the string value listed as the argument)
It is true this function method is not payable, but I am also not sending any value to it. (Value is 0). Additionally I think there might be a bug because the argument is listed as the function selector itself, which doesn't make sense to me.

Thoughts?

@ricmoo
Copy link
Member

ricmoo commented May 22, 2020

I think this is a bug. I check whether value is defined, and if so, fail. I should add an additional check, that if it is defined, that is is zero.

For now, removing the value should fix it and I’ll add a fix tomorrow.

The difference from v4 to v5 that is probably affecting you is that in v4, if the abi didn’t specify payable or mutabilityState it would default to payable (old Solidity semantics) whereas in v5, it defaults to non-payable (modern Solidity semantics).

@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. labels May 22, 2020
@crazyrabbitLTC
Copy link
Author

"The difference from v4 to v5 that is probably affecting you is that in v4, if the abi didn’t specify payable or mutabilityState it would default to payable (old Solidity semantics) whereas in v5, it defaults to non-payable (modern Solidity semantics)."

Is this going to be a problem going forward? I'm building a generalized factory for code that will generate front ends for any smart contract. I don't have the ability to control what version of Solidity the users smart contracts are written in.

Thanks!

@ricmoo
Copy link
Member

ricmoo commented May 23, 2020

If they are using a compiler /that/ old, they will likely have a lot of unaddressed security issues to deal with too.

They can easily resolve this though, by adding stateMutability to “payable” to all their functions though.

So, it provides additional safety to anyone using a contract written in the last 3-ish years, but there are simple solutions for people using ABIs generated older than that.

@ricmoo
Copy link
Member

ricmoo commented May 30, 2020

This should be fixed in 5.0.0-beta.189. Please try it out and let me know if you are still having problems.

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. bug Verified to be an issue. and removed on-deck This Enhancement or Bug is currently being worked on. investigate Under investigation and may be a bug. labels May 30, 2020
@crazyrabbitLTC
Copy link
Author

crazyrabbitLTC commented May 30, 2020

Seems to work! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

2 participants