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

Contract with function overloading #407

Closed
marcelomorgado opened this issue Jan 28, 2019 · 24 comments
Closed

Contract with function overloading #407

marcelomorgado opened this issue Jan 28, 2019 · 24 comments
Labels
discussion Questions, feedback and general information.

Comments

@marcelomorgado
Copy link

Hi,

A contract that I'm testing has different interfaces for same function name:

// Order that appers on ABI
initialize(string _name, string _symbol);
initialize(string _name, string _symbol, address _registry);
initialize(string _sender);
initialize();

I'm getting the error: Error: incorrect number of arguments when I'm trying call the second function.
Seems that when Contract is reading the ABI and it's only considering the first interface and ignoring the others.

// Expose the first (and hopefully unique named function)

errors.warn('WARNING: Multiple definitions for ' + name);

Notes:

  • I'm receving "WARNING: Multiple definitions" as well.
  • With web3js I can call all of interfaces.

Thanks!

@ricmoo
Copy link
Member

ricmoo commented Jan 28, 2019

If you are using function overloading, the first definition gets the "bare" name. You will need to specify the specific function you wish to call, for example:

// initialize(string _name, string _symbol);
contract.initialize(name, symbol).then( ... );
contract["initialize(string,string)"](name, symbol).then( ... );

// initialize(string _name, string _symbol, address _registry);
contract["initialize(string,string,address)"](name, symbol, address).then( ... );

// initialize(string _sender);
contract["initialize(string)"](sender).then( ... );

// initialize();
contract["initialize()"]().then( ... );

You should be able to print contract.functions as well to get a list of all available functions, overloaded or otherwise.

This is more of a limitation of un-typed languages (well, any language which doesn't have at least the subset of types that Solidity has). Make sense? :)

@ricmoo ricmoo added the discussion Questions, feedback and general information. label Jan 28, 2019
@marcelomorgado
Copy link
Author

Hi @ricmoo thanks for response,

  1. contract.functions works and using your code I've was able to call them. Maybe it'll solve my specific problem. Thanks.

  2. I'm not sure if is a javascript limitation because web3js works on the same scenario. In the case where we have two functions with the same name, the same number of params and different types, I agree with you, but itsn't the case.

I've written a sample contract with some test cases:

SampleContract.sol

pragma solidity ^0.5.0;

contract SampleContract {
  
  function overloading() public pure returns(uint) {
    return 1;
  }

  function overloading(string memory a) public pure returns(uint) {
    return 2;
  }

  function overloading(string memory a, string memory b) public pure returns(uint) {
    return 3;
  }
}

SampleContrat.test.js

describe.only("", () => {
    // OK
    it("should call overloading functions - web3js", async function() {
      const sampleContractWeb3 = new web3.eth.Contract(abi, address);

      const f1 = await sampleContractWeb3.methods.overloading().call();
      const f2 = await sampleContractWeb3.methods.overloading("a").call();
      const f3 = await sampleContractWeb3.methods.overloading("a", "b").call();

      expect(f1).to.equal("1");
      expect(f2).to.equal("2");
      expect(f3).to.equal("3");
    });

    // OK
    it("should call overloading functions - ethers", async function() {
      const provider = new ethers.providers.JsonRpcProvider();
      const sampleContractEthers = new ethers.Contract(address, abi, provider);

      const f1 = await sampleContractEthers["overloading()"]();
      const f2 = await sampleContractEthers["overloading(string)"]("a");
      const f3 = await sampleContractEthers["overloading(string,string)"](
        "a",
        "b"
      );

      expect(f1.toNumber()).to.equal(1);
      expect(f2.toNumber()).to.equal(2);
      expect(f3.toNumber()).to.equal(3);
    });

    // FAIL
    it("should call overloading functions - ethers", async function() {
      const provider = new ethers.providers.JsonRpcProvider();
      const sampleContractEthers = new ethers.Contract(address, abi, provider);

      const f1 = await sampleContractEthers.overloading();  // Error: incorrect number of arguments
      const f2 = await sampleContractEthers.overloading("a");
      const f3 = await sampleContractEthers.overloading("a", "b");

      expect(f1.toNumber()).to.equal(1);
      expect(f2.toNumber()).to.equal(2);
      expect(f3.toNumber()).to.equal(3);
    });
  });

Tests Output

> npx truffle test

Using network 'development'.



  Contract: SampleContract
    
      ✓ should call overloading functions - web3js (79ms)
WARNING: Multiple definitions for overloading
WARNING: Multiple definitions for overloading
      ✓ should call overloading functions - ethers (84ms)
WARNING: Multiple definitions for overloading
WARNING: Multiple definitions for overloading
      1) should call overloading functions - ethers
    > No events were emitted


  2 passing (240ms)
  1 failing

  1) Contract: SampleContract
       should call overloading functions - ethers:
     Error: incorrect number of arguments
      at Contract.overloading (node_modules/ethers/contract.js:130:19)
      at Context.<anonymous> (test/SampleContract.js:85:45)

"ethers": "^4.0.23",

@ricmoo
Copy link
Member

ricmoo commented Jan 29, 2019

So, there are certain cases which can be unambiguous, but that is mores the exception than the rule. A bigger problem comes when you wish to start passing overrides in, for example:

let overrides = {
    gasLimit: 100000
};
let response = contract.overloading(someValue, overrides);

In this case, it becomes more difficult to detect whether this is the 2-parameter signature with no overrides, or the 1-parameter signature with overrides. Type inspection can give you some additional non-ambiguous abilities, however, in the case that the second parameter to overloading were a tuple, you would be back in the same boat, since a struct Thing { uint256 gasLimit } could be a valid input to a contract.

So, rather than trying to guess, and sometimes being able to and other times not being able to, which would confuse the developer even more, I error on the side of non-ambiguity. In v5, overloaded operations will not be exposed. (correction: this only applies to ambiguous bare named overloaded operations; see below).

You can imagine things get more confusing when you have:

contract Foo {
    // In JavaScript a BigNumberish is the same regardless
    function bar(uint256 a) { }
    function bar(uint8 a) { }

    // In JavaScript, an address is a string...
    function bar(string a) { }
    function bar(address a) { }

    // In JavaScipt "0x12" could be a 4 character string, or a 1 byte array
    function bar(string a) { }
    function bar(bytes a) { }    
}

This is another area Web3 and I have disagreed on though. Guessing has often led to either significantly more code, or unexpected dapp security exploits though. :)

@marcelomorgado
Copy link
Author

Oh, thanks for the explanation!
Got it. I think that isn't an issue from your side. I'll need to think more about my implementation.
Thanks.

@d14na
Copy link

d14na commented Feb 18, 2019

You can imagine things get more confusing when you have...

@ricmoo I don't really feel as though the example you give in contract Foo is realistic. I have never seen a contract attempt to overload a function in this way (where you simple change the type of a param).

The example that @marcelomorgado gives in contract SampleContract where the number of params change is far more common. We have similar situations where we quite often offer 2 different versions of the same function, depending on who is calling owner or admin (admin will add the extra address _owner param).

FYI.. For a while now, I've noticed problems with the way web3js handles contracts in apps built with react(-native) and vue. Ethers.js has always "saved the day" (so really very grateful), but I'm worried about what this means In v5, overloaded operations will **not** be exposed. Will there still be a way to call overloaded function in v5?

Thanks!

Just want to add...

So, there are certain cases which can be unambiguous, but that is mores the exception than the rule.

as I mentioned above, I actually believe this to be the opposite, based on all the open-source solidity code I've read.

A bigger problem comes when you wish to start passing overrides in...

This seems to be the issue. Is there anyway to solve this with type casting, ie with TypeScript? I'm fairly new to TS, so I'm really not sure how that would work.

@ricmoo
Copy link
Member

ricmoo commented Feb 21, 2019

@d14na I have seen a few contracts that use overloading to accept different types of parameters during code audits. I agree it isn't common, but it is done, and would need to be handled. It is very confusing why something works in a bunch of cases, but in others it "just doesn't", and its because of an unrelated method to what you are dealing with having a name collision.

Oh! Totally sorry, I should have used a semi-colon in that sentence , it should read: "I error on the side of non-ambiguity; in v5, [ambiguous bare named] overloaded operations will not be exposed", i.e. only in the case of ambiguity. If there is only foobar(address), you can use contract.foobar(addr) or contract["foobar(address)"](addr). If there is foobar(address) and foobar(uint256), you would have to use contract["foobar(address)"](addr), there would be no contract.foobar (i.e. access by name).

Make sense? Sorry for the confusion. :)

There are ways to solve it, but they are all terrible, and some would fail for people using the JavaScript library directly without using TypeScript. I think this is the least terrible solution.

One suggestion being put forward is to expose "type indicator wrappers"... You could for example use contract.foobar(Uint256(value), Overrides({ gasPrice: 9000000000 })) vs contract.foobar(Address(addr)) for example, but that to me (at this point), seems alike a lot of extra typing and room for error for very little gained.

I'm always open to discussion though. :)

@d14na
Copy link

d14na commented Feb 22, 2019

There are ways to solve it, but they are all terrible, and some would fail for people using the JavaScript library directly without using TypeScript. I think this is the least terrible solution.

I agree. I'm always annoyed when software fails "silently", because it leads to inconsistencies that are extremely hard to diagnose for the uninitiated.

I have 4 WARNINGS that appear each time I load a new (rather complex) contract i'm working on (which is what brought me here); but they couldn't bother me any less, I just wanted to make sure that Ethers would remain a viable option for such uses past v4, which you've confirmed. Thanks so much for creating an amazing alternative to Web3js.

Keep up the great work!

@ricmoo
Copy link
Member

ricmoo commented Feb 22, 2019

Thanks! Glad you enjoy it. :)

As a quick note, ethers.errors.setLogLevel(“error”) will silence those warnings if they do start to get on your nerves. :)

@PaulRBerg
Copy link

ethers.errors.setLogLevel(“error”) doesn't seem to be working in the beta version anymore. Using 5.0.0-beta.162.

@ricmoo
Copy link
Member

ricmoo commented Dec 2, 2019

Oh yes. Each library has its own logger in v5 and I haven’t added the global flag yet.

I’ll be adding it soon.

@PaulRBerg
Copy link

PaulRBerg commented Dec 2, 2020

How would one use encodeFunctionData (part of Contract.interface) for an overloaded function? I'm getting this error:

Error: multiple matching functions

@ricmoo
Copy link
Member

ricmoo commented Dec 2, 2020

You must pass in the fully qualified signature. e.g. "setAddr(bytes32,uint256,bytes)" or "setAddr(bytes32,address)". You cannot use "setAddr" in this case.

If using the contract, the fully qualified signature must be normalized, but if using the Interface.encodeFunctionData, you can pass in and valid Fragment parameter (i.e. you may include memory modifiers, or other superfluous data like variable names).

@SvenMeyer
Copy link

Any chance that a TypeScript approach may provide a better solution ?

@zemse
Copy link
Collaborator

zemse commented Dec 24, 2020

@SvenMeyer Can you tell what version of @typechain/ethers-v5 you're using? 5.0.0 is the latest and I just confirmed it doesn't have the problem (I don't remember when/which version this was fixed).

Anyways you can still be able to use following in your codebase.

await this.stake.connect(this.signers.admin)['getStakedAmount()']();

@SvenMeyer
Copy link

@zemse Actually I just created my project 7 days ago from Paul Bergs' template
https://github.com/paulrberg/solidity-template

$ yarn list --depth 0 | grep ethers 
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @nomiclabs/[email protected]
├─ @typechain/[email protected]
├─ [email protected]

I tried the "alternative function call", it works, but ideally I would not confuse front end dev with another notation ...

@zemse
Copy link
Collaborator

zemse commented Dec 25, 2020

You can try upgrading @typechain/ethers-v5 and typechain packages to the latest ones in your project. There is just a one line change (PR under review) in the template.

Also, do you get type suggestions in like this? The name collision functions do not have a short name entry (getStakedAmount), only having function signature named functions.

Screenshot 2020-12-25 at 4 32 05 PM

@SvenMeyer
Copy link

@zemse I created a new project based on @PaulRBerg template https://github.com/paulrberg/solidity-template

https://github.com/SvenMeyer/paulrberg-solidity_template

created a second version of greet function with a different signature

    function greet() public view returns (string memory) {
        return greeting;
    }

    function greet(string memory message) public pure returns (string memory) {
        return (message);
    }

Test looks like this

    expect(await this.greeter.greet()).to.equal("Hello, world!");
    expect(await this.greeter.greet("New Message")).to.equal("New Message");

... and I still I get an error

$ yarn test
yarn run v1.22.10
$ hardhat test
Creating Typechain artifacts in directory typechain for target ethers-v5
Successfully generated Typechain artifacts!


  Unit tests
    Greeter
Deploying a Greeter with greeting: Hello, world!
      1) should return the new greeting once it's changed


  0 passing (602ms)
  1 failing

  1) Unit tests
       Greeter
         should return the new greeting once it's changed:
     TypeError: this.greeter.greet is not a function
      at Context.<anonymous> (test/Greeter.behavior.ts:5:31)
      at step (test/Greeter.behavior.ts:33:23)
      at Object.next (test/Greeter.behavior.ts:14:53)
      at /home/sum/DEV/ETH/PaulRBerg/paulrberg-solidity_template/test/Greeter.behavior.ts:8:71
      at new Promise (<anonymous>)
      at __awaiter (test/Greeter.behavior.ts:4:12)
      at Context.<anonymous> (test/Greeter.behavior.ts:43:16)

error Command failed with exit code 1.


$ yarn list --depth 0 | grep ethers
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @ethersproject/[email protected]
├─ @nomiclabs/[email protected]
├─ @typechain/[email protected]
├─ [email protected]

@zemse
Copy link
Collaborator

zemse commented Dec 29, 2020

Ethers.js does not create a greet function for you (like web3js does maybe), so you have to explicitly use the fully qualified signature (e.g. this.greet['greet()']) for the function you are intending to pick.

The reason is available in an earlier comment.

@SvenMeyer
Copy link

@zemse thanks for clarifying (again). I thought that with the update, ethers would just provide all the "right" functions in the standard call format.

@zemse
Copy link
Collaborator

zemse commented Dec 29, 2020

No worries.. I thought you were looking for a typescript solution which does tells you that a this.greeter.greet() is incorrect, right when you are typing it.

@ricmoo
Copy link
Member

ricmoo commented Dec 29, 2020

I thought that with the update, ethers would just provide all the "right" functions in the standard call format.

The problem is there is no “right” function; either could be what you wanted. :s

@livingrock7
Copy link

livingrock7 commented Aug 19, 2021

@ricmoo
How do I programmatically identify overriding functions from ABI and do this

we have generic code below where it just takes methodName
gasLimit = await contract.estimateGas[methodName](...paramArray, { from: account });

does it make sense to always use methodName+(++) ? Is there a better way?

@grapevinegizmos
Copy link

I'm not completely following this thread, but my tests (using Hardhat) of a strings library are failing for failure to discriminate between two same name functions which differ in number of parameters. I have been using this method to supply sensible default values in functions where appropriate, for example the findChar function which finds the 1-based position in a string of the occurrence of a character, optionally starting after a specified position.

I use this function signature for the "primary version"
function findChar(string memory source, string memory char, uint startAfter) public pure returns(int) {..}
but I provided the one below so that if you don't specify startAfter it assumes 0;

function findChar(string memory source, string memory char) public pure returns(int) { return findChar(source, char, 0); }

This works as expected in truffle and remix, but fails in hardhat I'm guessing cause of the difference between ethers and web3, I get a type error ("is not a function") no matter how I call it, which I can't get to go away except by deleting the convenience version (with only two params). Is really the intention? Seems a shame to lose the ability to test overloaded functions which is one of the relatively few convenience features of solidity that makes life easier.

@wschwab
Copy link

wschwab commented Nov 1, 2021

I'm also getting issues in Hardhat's Waffle with ethers on an overloaded function (safeTransferFrom on ERC721). Strangely, when doing an event test it works, but not otherwise. The structure of event tests in Hardhat Waffle is:

await expect(contract.function(args).to.emit(contract, 'EventName')

which puts an await outside the expect, whereas the failing calls are all of a simpler await like:

await contract.function(args);

I don't know why that should make a difference, but figured I'd mention it just in case it does.

koonopek added a commit to redstone-finance/redstone-oracles-monorepo that referenced this issue Aug 12, 2024
* feat: extend from multicall3 in redstoneMulticall3

* fix: redeclaration of type

* fix: use only single aggregate3 function - ethers-io/ethers.js#407
koonopek added a commit to redstone-finance/redstone-oracles-monorepo that referenced this issue Sep 24, 2024
* feat: extend from multicall3 in redstoneMulticall3

* fix: redeclaration of type

* fix: use only single aggregate3 function - ethers-io/ethers.js#407
koonopek added a commit to redstone-finance/redstone-oracles-monorepo that referenced this issue Oct 3, 2024
* feat: extend from multicall3 in redstoneMulticall3

* fix: redeclaration of type

* fix: use only single aggregate3 function - ethers-io/ethers.js#407
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Questions, feedback and general information.
Projects
None yet
Development

No branches or pull requests

9 participants