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

Support for a custom key in TransactionRequest and CallOverrides #1761

Closed
ly0va opened this issue Jul 9, 2021 · 3 comments
Closed

Support for a custom key in TransactionRequest and CallOverrides #1761

ly0va opened this issue Jul 9, 2021 · 3 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.

Comments

@ly0va
Copy link

ly0va commented Jul 9, 2021

Currently, Signer.checkTransaction and contracts.populateTransaction reject custom override keys to prevent typos or unsupported options as seen here and here.

While the former method can be overridden by a derived class, there is no such option for the latter, as it is not a method of any class. Moreover, it is not exported, so there is no way to utilize the existing implementation without copy-pasting it.

The problem here is that when extending a Signer or a Wallet to be used to call contracts on networks that also use web3 API, but have additional options that must be specified to modify transaction calldata prior to sending, there is no way to pass those options into Signer.signTransaction or other methods that accept a TransactionRequest from the context of a contract, since contracts.populateTransaction used by the Contract class does not allow stray CallOverrides.

To give an example, I want to be able to do something like

// mylib.Wallet extends ethers.Wallet and overrides signTransaction and checkTransaction
// methods to be suitable for a custom network that needs some custom options to modify & sign transactions
const wallet = mylib.Wallet(key, provider); 

// contract uses wallet's overridden methods to build calldata
const contract = new ethers.Contract(address, abi, wallet);

// custom overrides are passed into wallet.signTransaction which will use them to modify the calldata
const tx = await contract.myFunction(
    ...args,    // regular arguments for the contract method
    {
        gasLimit: 30000,    // standard override
        custom: { customField: "abacaba" }  // custom override
    }
);

which is currently impossible due to checks in contracts.populateTransaction which I linked above.

Alternatively, the way to do this is by extending ethers.Contract using a lot of copy-pasted code to override Contract.populateTransacton, which is not pretty.
Adding a single custom field (or any other suitable name) would be a major help and will not compromise typo checks that are in place.

Maybe there is a much simpler way to do this which I am not seeing, in which case I would be very grateful for suggestions.
If not, then my suggested way is fairly straightforward to implement and I would gladly make a PR if this solution looks fine.

@ly0va
Copy link
Author

ly0va commented Jul 12, 2021

To add one more argument for this change, if there is no way to specify custom options, then there is no point in CallOverrides and PopulatedTransaction being interfaces and not concrete types. Which, I guess, are made interfaces to provide more flexibility, which is then, unfortunately, denied.

@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. labels Jul 20, 2021
@ricmoo ricmoo added the minor-bump Planned for the next minor version bump. label Aug 24, 2021
@Madeindreams
Copy link

Madeindreams commented Oct 21, 2021

I'm also looking into adding custom argument to transactions.

But right now I'm experimenting. I'm trying to checkTransaction on a custom transaction. I am getting the address of the signer back but it's wrapped in an a promise that I can't await for.

const publicKey = wallet.checkTransaction(hashTx,this.signature)

output  { from: Promise { '0xAbC123....' } }

Even if I await for it inside a promise it always resolve to this. So I was wondering if that could be because I have custom attributes.

EDIT: Nope it has nothing to do with it. I have to await for
wallet.checkTransaction(hashTx,this.signature).from

@ricmoo
Copy link
Member

ricmoo commented May 12, 2022

This was added in 5.5.0, so I'll close this now. :)

@ricmoo ricmoo closed this as completed May 12, 2022
@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. labels Jun 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published. minor-bump Planned for the next minor version bump.
Projects
None yet
Development

No branches or pull requests

3 participants