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

Typed Transactions #1364

Closed
ricmoo opened this issue Mar 15, 2021 · 14 comments
Closed

Typed Transactions #1364

ricmoo opened this issue Mar 15, 2021 · 14 comments
Labels
documentation Documentation related issue. enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@ricmoo
Copy link
Member

ricmoo commented Mar 15, 2021

I've already completed most of the work on this, I'm just testing now, but am creating this issue for tracking purposes.

The related EIPs are:

This will need a change to the interface, but as all new properties are optional, this should not be too much of an issue.

@ricmoo ricmoo added enhancement New feature or improvement. on-deck This Enhancement or Bug is currently being worked on. labels Mar 15, 2021
ricmoo added a commit that referenced this issue Mar 30, 2021
@ricmoo
Copy link
Member Author

ricmoo commented Mar 30, 2021

The initial support for this has been added in 5.1.0.

Currently AlchemyAPI and Pocket do not fully/properly support EIP-2930 transactions, and Etherscan has limited support for it. If you need typed transactions, please use INFURA (via the InfuraProvider is fine).

As these services update their nodes and fix their configurations, I will turn them back on and push out newer versions of the 5.1 packages.

Please try them out though, and let me know if there are any issues! Thanks! :)

@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 Mar 30, 2021
@fvictorio
Copy link

Minor feedback on this: transaction receipts for typed transactions have a type field. The JSON-RPC in geth returns it, and we are going to do the same in Hardhat.

@fvictorio
Copy link

More feedback (although this one is more opinionated): doing getTransaction with a legacy transaction returns an accessList: null. I think it would be better to remove it altogether in those cases.

@ricmoo
Copy link
Member Author

ricmoo commented Apr 10, 2021

@fvictorio I’ll double check this in a week or so, but I don’t see the type field coming back in the receipts on all backends.

But this far there has been pretty wide-spread inconsistent behaviours between even the most popular services. I expect this to be resolved more quickly once Berlin is on mainnet.

Obviously, that is not ideal, but for now I am keeping it off the TransactionReceipt object, so that all backends continue to match. I’ll add it once the popular third-party services agree again.

Etherscan currently still incorrectly calculates estimateGas, but I’m trying to reach out to them to get that fixed. Once that is all straightened out, I’ll reiterate over all the services to make sure their output matches and contact the ones that don’t again.

Berlin has been exhausting. :p

@ricmoo
Copy link
Member Author

ricmoo commented Apr 10, 2021

I think I prefer to keep type and accessList as null for typed transactions, but I will think about that more. I’m not hard-set on that though, so feel free to make arguments to sway me. It was a deliberate decision though. :)

Whatever choice is made will continue on with EIP-1559.

One motivating reason is the move to strictNullChecks in v6, so I can use things like type: null | number. If I remove it, then it becomes type: null | undefined | number or needs to be set optionally (i.e. type?), which I’m trying to reduce in result types, which are also moving to concrete classes instead of just objects with properties.

@jamesmorgan
Copy link

Are there any docs on the right way to use these changes, specifically the accessList changes? I cant find anything on the docs

@ricmoo
Copy link
Member Author

ricmoo commented Apr 15, 2021

Great point. I will add information to the docs today.

@ricmoo ricmoo added the documentation Documentation related issue. label Apr 15, 2021
@fvictorio
Copy link

Are there any docs on the right way to use these changes, specifically the accessList changes? I cant find anything on the docs

I've been using ethers to test access list transactions and just passing it has worked fine:

myContract.myFunction(arg1, arg2, {
  accessList: [{
    address: "0x...",
    storageKeys: ["0x...", "0x...", ...]
  }, { ... }, ...]
})

If you are using a node that supports eth_createAccessList, you'll need to call that method directly. I don't know how you call a "raw" RPC method, but in hardhat you would do something like this:

const result = await network.provider.send("eth_createAccessList", { from, to, data, ... })
// result.accessList can be used as a parameter for ethers

Not saying that docs are not needed, I'm just commenting in case anyone can't wait to use this 😅

@jamesmorgan
Copy link

jamesmorgan commented Apr 17, 2021

Thanks for the info, looks simple enough @fvictorio.

I was wondering if there would be helper methods like estimateGas, something along these lines e.g.

const gasLimit = await myContract.myMethod.estimateGas(data, {from, to});
const accessList = await myContract.myMethod.createAccessList(data, {from, to});
const tx = await myContract.myMethod(data, {
      from: account,
      gasLimit,
      accessList
    })

(sample code only 😄 )

@ricmoo
Copy link
Member Author

ricmoo commented Apr 18, 2021

Docs have been update. I've added the field to the TransactionRequest (which accepts any AccessListish) and TransactionResponse (which will contain a normalized AccessList) as well as documented the accessListify method, which allows others to normalize AccessListish into an AccesList.

I have experimented with eth_createAccessList, and have found it is not very widely/well supported on deployed backends right now. In the future I may add this, but for now I will probably try to make a utility function which will call that on a JsonRpcProvider.

Ideally, it is something that would preferably be handled within a Signer (with the option to explicitly override), using logic akin to:

const [ untypedEstimate, typedEstimate ] = await Promise.all([
    provider.estimateGas(untypedTx),
    provider.estimateGas(typedTx),
]);

if (untypedEstimate.lt(typedEstimate)) {
    this.sendTransaction(untypedTx);
} else {
    this.sendTransaction(typedTx);
}

But time will tell. :)

@jamesmorgan
Copy link

Thanks for updating the docs @ricmoo 👍🏻 .

I have also been playing around with eth_createAccessList and doesnt look like it has wide support yet which is a shame.

Would be great to get that utility method in place for when it does.

@ricmoo
Copy link
Member Author

ricmoo commented May 30, 2021

Closing this for now. Hopefully once createAccessList has more adoption we can re-address it… :)

@ricmoo ricmoo closed this as completed May 30, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
pull bot pushed a commit to shapeshift/ethers.js that referenced this issue Jun 4, 2021
@Lucienest
Copy link

How it'd get widespread adoption if you decline to add support for it?

@ricmoo
Copy link
Member Author

ricmoo commented Feb 28, 2022

What do you mean? Typed Transactions are supported.

It’s eth_createAccessList that is not widely supported by most nodes I tried at the time. Most users of access lists do not need the RPC end-point, as they have specific slots and addresses known that need access (i.e. Proxy contracts which rely on the 2300 gas stipend).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation related issue. enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

4 participants