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

Misleading error message: "Can not send value to non-payable contract method or constructor" #2488

Closed
gskapka opened this issue Mar 11, 2019 · 4 comments · Fixed by #2513
Closed
Labels
Bug Addressing a bug

Comments

@gskapka
Copy link

gskapka commented Mar 11, 2019

Description

Receive "Can not send value to non-payable contract method or constructor instead!" error when calling a function that is definitely payable. Also, the error itself grammatically makes no sense. Also, the error itself makes no sense since the method is payable.

Expected behavior

Shouldn't receive error about non-payable method or constructor if the method or constructor is payable.

Actual behavior

Returns above error.

Steps to reproduce the behavior

Works fine with web3@1.0.0-beta.47, but throws aforementioned error in ...-beta.48.

Using the following solidity in order to query an offchain API using Provable (Oraclize):

pragma solidity ^0.5.0;

import "./oraclizeAPI.sol";

contract DieselPrice is usingOraclize {

    uint public dieselPriceUSD;

    event LogNewDieselPrice(string price);
    event LogNewOraclizeQuery(string description);

    constructor() public {
        update(); // First check at contract creation...
    }

    function __callback(bytes32 myid, string memory result) public {
        require(msg.sender == oraclize_cbAddress());
        emit LogNewDieselPrice(result);
        dieselPriceUSD = parseInt(result, 2); // Let's save it as cents...
        // Now do something with the USD Diesel price...
    }

    function update() public payable {
        emit LogNewOraclizeQuery("Oraclize query was sent, standing by for the answer...");
        oraclize_query("URL", "xml(https://www.fueleconomy.gov/ws/rest/fuelprices).fuelPrices.diesel");
    }
}

And the following javascript to test that the update() function should fail if the contract's balance is zero (because Provable [Oraclize]) requires a fee):

  it('Should revert on second query attempt due to lack of funds', async () => {
    const expErr = 'revert'
    try {
      await methods
        .update()
        .send({
          from: address,
          gas: gasAmt
        })
      assert.fail('Update transaction should not have succeeded!')
    } catch (e) {
      assert.isTrue(
        e.message.startsWith(`${PREFIX}${expErr}`),
        `Expected ${expErr} but got ${e.message} instead!`
      )
    }
  })
}

...results in the above error. Tests pass as expected with beta47 but not beta48. Worse, the error message makes no sense since it's clear there is:

  • a) No value being sent in the methods.update().send(...params) function
  • b) The update() method in the contract is payable.

Error Logs

"Can not send value to non-payable contract method or constructor instead!"

Full Example

You can find the above Provable (Oraclize) example with Truffle tests in the repo here. Simply pull it, install its dependencies and then install web3@1.0.0-beta.48 & follow the README to run the tests.

Versions

  • web3.js: -beta.48
  • nodejs: 10.15.0
  • ethereum node: Ganache/TestRPC
@nivida nivida added the Needs Clarification Requires additional input label Mar 11, 2019
@nivida
Copy link
Contributor

nivida commented Mar 11, 2019

"Can not send value to non-payable contract method or constructor instead!"

This error message does not exist in the web3.js project the error message would be:
"Can not send value to non-payable contract method or constructor"

Works fine with web3@1.0.0-beta.47, but throws aforementioned error in ...-beta.48

The difference between version beta.48 and beta.47 is just a fix for the custom provider handling and the return value handling of a contract call. I've tested it with a freshly deployed contract and called several methods without any error popping up.

@OFRBG
Copy link

OFRBG commented Mar 11, 2019

I can confirm this behavior. @gskapka did misread the test result. The error is in fact Can not send value to non-payable contract method or constructor, and "instead" is part of the assert:

`Expected ${expErr} but got ${e.message} instead!`

I tested with beta.47 and it threw the same message, however. @nivida So it is highly likely that the problem arose with some other release.

@gskapka
Copy link
Author

gskapka commented Mar 12, 2019

I can confirm this behavior. @gskapka did misread the test result

Oops! Hoisted by my own petard! Glad @OFRBG could confirm the behavioural difference however.


The problem clarified:

Receive Can not send value to non-payable contract method or constructor error when calling a contract method that is payable, whilst also sending no value.

@gskapka gskapka changed the title Cryptic error message: "Can not send value to non-payable contract method or constructor instead!" Misleading error message: "Can not send value to non-payable contract method or constructor" Mar 12, 2019
@nivida nivida added Bug Addressing a bug and removed Needs Clarification Requires additional input labels Mar 13, 2019
@nivida
Copy link
Contributor

nivida commented Mar 13, 2019

Thanks for the additional details! I will fix and release it asap.

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

Successfully merging a pull request may close this issue.

3 participants