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

Fix: Make Interface.from() Full Clone #4689

Closed
wants to merge 1 commit into from

Conversation

adraffy
Copy link

@adraffy adraffy commented Apr 14, 2024

Interface.format("json") in v6 implies "minimal" which causes Interface.from() to drop a lot of meta information.

Instead, arg-less format() should be called.

In v5, it's safe to call arg-less format(), as the default is Full
https://github.com/ethers-io/ethers.js/blob/555f16ce7bc6f5fcb40f4c88037f3e4e4182c36c/

// Maybe an interface from an older version, or from a symlinked copy
if (typeof((<any>value).format) === "function") {
    // OLD:
    //return new Interface((<any>value).format("json"));

    // NEW:
    return new Interface((<any>value).format());
}

Alternatively, this could do an additional check for formatJson() from v6.

@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. v6 Issues regarding v6 next-patch Issues scheduled for the next arch release. labels Apr 17, 2024
@ricmoo ricmoo added bug Verified to be an issue. and removed investigate Under investigation and may be a bug. labels Apr 25, 2024
@ricmoo
Copy link
Member

ricmoo commented May 1, 2024

This should only happen when using 2 different versions of v6, since the first check is abi instance Interface, but I think it still makes sense to make that check, since obviously it came up for you at some point, so might sneak up on others too. :)

@ricmoo
Copy link
Member

ricmoo commented May 1, 2024

Fixed in v6.12.1. Let me know if you still have any issues.

Thanks! :)

@ricmoo ricmoo closed this May 1, 2024
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. next-patch Issues scheduled for the next arch release. labels May 1, 2024
@adraffy
Copy link
Author

adraffy commented May 1, 2024

This should only happen when using 2 different versions of v6, since the first check is abi instance Interface, but I think it still makes sense to make that check, since obviously it came up for you at some point, so might sneak up on others too. :)

Yeah it was rare, I encountered it importing between two projects (where I'm testing local changes in one NPM package from another package before release.) Thanks for the fix!

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. v6 Issues regarding v6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants