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

function overriding error #1432

Closed
TheGreatHB opened this issue Apr 6, 2021 · 12 comments
Closed

function overriding error #1432

TheGreatHB opened this issue Apr 6, 2021 · 12 comments
Labels
fixed/complete This Bug is fixed or Enhancement is complete and published. investigate Under investigation and may be a bug.

Comments

@TheGreatHB
Copy link

G'day, mates!

While I'm testing Strings.sol library, I got an error with TypeError: uniqueNames[name_1].push is not a function.

contracts

library Strings {
    function toString(uint256 value) public pure returns (string memory) {
        if (value == 0) {
            return "0";
        }
        uint256 temp = value;
        uint256 digits;
        while (temp != 0) {
            digits++;
            temp /= 10;
        }
        bytes memory buffer = new bytes(digits);
        while (value != 0) {
            digits -= 1;
            buffer[digits] = bytes1(uint8(48 + uint256(value % 10)));
            value /= 10;
        }
        return string(buffer);
    }
}

testing script

const { expect, assert } = require("chai");
const { ethers } = require("hardhat");

describe("Library Test", function () {

    let Test, test, owner, addr1;

    beforeEach(async function () {
        Test = await ethers.getContractFactory("Strings");
        [owner, addr1] = await ethers.getSigners();
        test = await Test.deploy();
    })

    it("Strings test", async function () {
        const t1 = await test.toString(123);                     //error point
        console.log(t1);
    });
});

I changed a function name from toString to toStrin just for testing. and it works.
function toString(uint256 value) public

function toStrin(uint256 value) public
                             &
const t1 = await test.toStrin(123);

So I thought it's because js has the function with the same name.
But although I tried callStatic function to override the js function, it still didn't work.

const t1 = await test.callStatic["toString(uint256)"](123);

repeatedly same error
TypeError: uniqueNames[name_1].push is not a function.

But expectedly const t1 = await test.callStatic["toStrin(uint256)"](123); with function function toStrin(uint256 value) public worked.

@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. labels Apr 10, 2021
@ricmoo
Copy link
Member

ricmoo commented Apr 10, 2021

I may need to use Object.hasOwnProperty internally, as I might be using a null check. I’ll look into this as soon as possible.

@Huge
Copy link

Huge commented Jun 1, 2021

Thanks, just happened to me yesterday, blame the SO: https://ethereum.stackexchange.com/a/58341/1210 .'-D
Hope there is some way around this, even explicit warning would be enough.

@k06a
Copy link

k06a commented Jun 2, 2021

This code was intended to be used as an internal library, not as a public methods :)

@tarrencev
Copy link

I ran into a similar issue with a contract function overloading a js object property (valueOf).

@tarrencev
Copy link

@ricmoo this diff should fix the issue. unfortunately i can't open a PR with it right now.

commit 6bbc63c92298ac7263450a11067c4331665dbfc5 (HEAD -> master)
Author: Tarrence van As <tarrence13@gmail.com>
Date:   Mon Jun 7 06:46:42 2021 -0700

    fix(contracts): solidity function names conflicting with js object property names

diff --git a/packages/contracts/src.ts/index.ts b/packages/contracts/src.ts/index.ts
index bec415f7..bc34b2bb 100644
--- a/packages/contracts/src.ts/index.ts
+++ b/packages/contracts/src.ts/index.ts
@@ -685,7 +685,7 @@ export class BaseContract {
             }
         }

-        const uniqueNames: { [ name: string ]: Array<string> } = { };
+        const uniqueNames = new Map<string, Array<string>>();
         const uniqueSignatures: { [ signature: string ]: boolean } = { };
         Object.keys(this.interface.functions).forEach((signature) => {
             const fragment = this.interface.functions[signature];
@@ -702,8 +702,8 @@ export class BaseContract {
             // are ambiguous
             {
                 const name = fragment.name;
-                if (!uniqueNames[name]) { uniqueNames[name] = [ ]; }
-                uniqueNames[name].push(signature);
+                if (!uniqueNames.has(name)) { uniqueNames.set(name, [ ]) }
+                uniqueNames.get(name).push(signature);
             }

             if ((<Contract>this)[signature] == null) {
@@ -730,10 +730,8 @@ export class BaseContract {
             }
         });

-        Object.keys(uniqueNames).forEach((name) => {
-
+        uniqueNames.forEach((signatures, name) => {
             // Ambiguous names to not get attached as bare names
-            const signatures = uniqueNames[name];
             if (signatures.length > 1) { return; }

             const signature = signatures[0];

@ricmoo
Copy link
Member

ricmoo commented Oct 5, 2021

@tarrencev Unfortunately, JavaScript `Map cannot be used in v5. In v6, more modern JavaScript features will be employed, but for v5 things must be kept a bit more primitive.

@ricmoo
Copy link
Member

ricmoo commented Oct 20, 2021

This should be fixed in 5.5.0. Please try it out and let me know.

Thanks!

@ricmoo
Copy link
Member

ricmoo commented Mar 17, 2022

This was fixed, so I'll close it. Please re-open or create a new issue if there are any problems.

Thanks! :)

@ricmoo ricmoo closed this as completed Mar 17, 2022
@AyyanNiazi
Copy link

Hey @ricmoo I'm getting same error when I'm deploying contract

@ricmoo
Copy link
Member

ricmoo commented Apr 8, 2022

@AyyanNiazi what version of ethers are you using? Can you include a code snippet and the ABI?

@hobbydev71
Copy link

Hi @ricmoo ,
I have same issue now on ethers 5.5.0.
What's the main reason of this issue?

@FortisFortuna
Copy link

Same issue

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

No branches or pull requests

8 participants