Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: use CallOverrides, which is more complete than PayableOverrides #345

Closed
wants to merge 1 commit into from

Conversation

liamzebedee
Copy link

@liamzebedee liamzebedee commented Mar 12, 2021

Temporary fix

ethers.js allows users to pass the from field in overrides, which is useful for contracts like WETH9.deposit({ from: accounts[3], value: '1' }).

Ideally, this block should be reworked, as all ethers.js function calls are able to accept the from parameter as an override (as shown here).

    !options.isStaticCall && !isConstant(fn) && !isConstantFn(fn)
      ? `overrides?: ${isPayable(fn) ? 'PayableOverrides' : 'Overrides'}`
      : 'overrides?: CallOverrides'
    'overrides?: CallOverrides'

ethers.js allows users to pass the `from` field in overrides, which is useful for contracts like
WETH9.deposit({ from: accounts[3], value: '1' }).
@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2021

⚠️ No Changeset found

Latest commit: 6b93a0f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@quezak
Copy link
Contributor

quezak commented Mar 12, 2021

Using CallOverrides is also invalid because calling a payable function shouldn't accept a blockTag override -- if we want to fix anything on our side we should create a separate interface that's correct.

If indeed all ethers calls accept a from in overrides param, the correct solution would be to post an issue in Ethers to add from to their PayableOverrides and/or Overrides interfaces.

@krzkaczor
Copy link
Member

This is needed indeed. connect doesn't work in the same way in some cases. I will try to figure out a fix in a sec.

@krzkaczor
Copy link
Member

For now a quick workaround would be to generate type with & { from: string | Promise<string> }.

I will create a separate issue to fix this (and few other things) in ethers.

@krzkaczor
Copy link
Member

Superseded by #350. Thanks for the original PR

@liamzebedee
Copy link
Author

Ah yeah, this is a better temporary solution @krzkaczor. Cheers!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants