-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 EIP-2718 transaction envelope types beyond 1 and 2 #3747
Comments
My first thought would be to sub-class Transaction, and in the static You can similarly override the The built-in functions for parsing and serializing aren’t really exposed, but they are pretty trivial to work with using the decodeRlp and encodeRlp. I’ve been considering adding custom network packages for non-standard networks. If you make any process, let me know and I can start that program earlier on. :) |
Thanks for the quick reply. I did some research a while back on Subclassing and as i went down the rabbit hole I kept needing to subclass and overwrite more methods. one issue was that copyRequest kept removing properties from the transaction object. Im not sure what a moon-standard network is (although googling i learned The Moon is getting a G4 cellular network) I hope it means that the work im gonna do is helpful beyond my case |
(updated: moon-standard -> non-standard) Lol! Oh. Autocorrect. I was contemplating the The reason I don’t want to ingrate anything too directly is that over the years, dozens of chains have wanted their “own logic” baked in. Many of those chains no longer exist (or I have never heard of them since). Some networks may use the same type for another format or such too. I’ve historically made separate packages for them, which various levels of success. For example, Aion required blake2b added because they use it for their hashes, which increases the library size for something no one else requires. Some networks required some large emscripten library of 1.2 megabytes of protocol buffers and whatnot just to compute the hash. So, I target Ethereum and chains that don’t deviate too far. Part of v6 is to try making it easier to support non-standard networks (I like the term “strange networks”, meant as a fun term, but comes off more often as mean. :p) So, I’m all ears if you have other suggestions. I think if |
Definitely respect that and that you have kept ethers as small as it can be. Ill Do some more research in the direction of enabling easier extension rather than adding specific features |
I’ll also add a feature in the v6.1 that makes it easier to support Transaction subclasses. |
What about adding a way to set delegators to handle different kind of txs. Something like, plug the type 0x14, and delegate it's calls to a specific class that has to cover the way you can serialize it, the extra data to be assigned and etc etc. I'm not saying that you are the only ones that should do that, haha, but maybe a "could ethersjs accept to add a module that allow others to plug their txs without changing their code" |
I did a read thru of code for spots where there could be issues. #3751 Maybe its helpful maybe its not. |
I was looking at the work done on Plugins and I think this could be a great way to handle extendability rather than Transaction subclasses as not every thing that needs to be extensible is part of that class and often most of the body for Transaction methods should remain.
|
The problem is Plugins are mainly asynchronous in nature. And the Transaction operations are all synchronous. You don't always have a Network object, but should still be able to process Transactions... But I was imagining, for a given network it would have code that looks like: import { ethers } from "ethers";
// This Transaction class is a sub-class of ethers.Transaction, and should be used when the
// envelope types are necessary
import { CeloNetwork, Transaction } from "@ethers-ext/celo";
// The CeloNetwork would certainly have a bunch of plugins though, to handle custom
// block properties, transactions, etc.
const provider = new JsonRpcProvider(url, CeloNetwork); The problem with global registration is that various bundlers and other tools really do not play well with global registration things. Especially if there are 2 different versions of ethers in use (such as when using hardhat for testing), as the registration only happens against the version where the registration happened explicitly, which then means it either works in the app and not in the tests or works in the tests and not in the app. A bigger issue, is that it is complicated to use in ESM projects, since // SetupCelo/index.js
import { ethers } from "ethers";
ethers.Network.networks.celo = "foobar";
// main.js
import "SetupCelo"
import { ethers } from "ethers";
// Things done here operate before all imports are SetupCelo is resolved
console.log(ethers.Network.networks.celo);
// undefined
setTimeout(() => {
// Now things are fine, but developers are (justifiable) confused... And also have to wrap
// all their code to execute in the event loop
console.log(ethers.Network.networks.celo);
// "foobar"
}, 0); There are a lot of little gotchas, that have gotcha-ed me over the years. :) |
Thanks for talking this out. 😊 I agree its best to avoid global registration.
Maybe I just haven't understood how you intend for plugins to be used but I don't see anything inherently async about them. I saw AbstractProvider has access to a attachPlugin function. I realized later that ProviderPlugins are different than NetworkPlugins. But had been thinking the Probably has issues still. 🤔 Do you think the idea of some object that given a tx type number can say ok "parse with this, serialize with this, these are the allowed/needed properties" is a good way to encapsulate allowing alternative tx types? ? |
I noticed that this issue was labeled with
and with the recent release of v6.1.0 if there were any updates on this issue? |
I implemented it the way I was originally planning and had to back it out because there were still some underlying issues with it, that make it more complex than I expected. It’s going to need NetworkPlugin support and more fine grained changes I need to experiment with, so it didn’t make it into v6.1. It’s still on my priority list though, which is why it’s still |
Hey, has there been any further updates on the above? |
Not recently, sorry. I’m still experimenting with the best way to add the functionality as a NetworkPlugin. I am doing a next-patch blitz this week, which means I’ll be revisiting the on-deck soon too. I may rename these tags as they are starting to get confusing as to their original meanings… |
+1 would also love to see EIP-2718 supported. Currently exploring alternative options. |
Check-inHey @ricmoo :) Brief follow up, are there any recent changes that would make it easier to support chain-specific transactions types in ethers v6? If not, is there anything we can do to help? Thanks in advance! Some history and context (in case it's useful)In June 2023, we (Celo engineers from cLabs 👋) proposed a design that would allow The PR was kindly accepted, and we were the first team to implement a chain-specific transaction type in
Since then, Optimism transaction types were also added: We repeatedly hear from developers that they would love to build applications with ethers and access to "custom gas currency transactions" supported on Celo. As far as we know, this is not possible so, unfortunately, we resorted to wrapping ethers and re-implementing a chunk of the library in the past to support custom transaction types: |
I had an idea on how to do this the other day, and will likely try an experimental branch to test it out, but curious on your thoughts. What if the This would allow Celo, for example to create a GasCurrencyType (or some similar name) function which could be passed in like: The function would returns a sub-class of Transaction, extended to support the additional transaction type. Feedback? :) |
Thanks @ricmoo so much for putting your effort into it! Your approach sounds really good. Just couple of clarifying questions:
I briefly tried it and seems to me that (given my limited understanding) changing the My suggestion would be to introduce a new field ( So let's say we would introduce a new transaction (just a descriptive example): class DynamicFeeCurrencyTransaction extends Transaction {
protected feeCurrency : string;
constructor(feeCurrency: string) {
super();
this.feeCurrency = feeCurrency;
}
get type(): null | number {
return 123;
}
get typeName(): null | string {
return "cip64";
}
get serialized(): string {
return '0xsigned';
}
get unsignedSerialized(): string {
return '0xunsigned';
}
} and then provide the const tx = await signer.sendTransaction({
type: 123,
to: "0xaddress",
value: 1n,
transactionFactory: (tx: TransactionLike): Transaction => {
const result = new DynamicFeeCurrencyTransaction("0x874069Fa1Eb16D44d622F2e0Ca25eeA172369bC1");
// populate all fields from tx object
return result;
}
}); Then typeof tx.transactionFactory === "function"
? tx.transactionFactory(pop)
: Transaction.from(pop); but my issue is that there's another call to No idea yet for signing (but I can imagine something similar could work here?). What do you think about that approach? |
We’d like to bring some extra attention to this discussion with the aim of getting a solid plan in place. Why do we think supporting fee currency feature is important in general? Custom gas fee currency feature allows projects and developers on Celo network to use whitelisted tokens to be used to pay for gas. It provides flexibility and seamless experience by not requiring users to hold native currency just to be able to pay for the network fees. While the feature has been there for a while and there are examples of successful projects using it like Valora (code example) and Impact Market (code example). MiniPay supports the feature as well and by reaching a 1 million users milestone it plays a huge role in adoption. It also has gained more traction recently with projects like GoodDollar (see discussion) and USDC (see discussion) are on the radar to possibly become gas currencies as well. Why do we think supporting the custom transaction serialization is important for ethers? In the ethereum realm, from the standard ethers/viem/web3 libraries, ethers is currently #1 library (based on the trends). If someone were to start a new multichain (or chain specific) project though they might ask themselves a question “How many chains can I support using a single library?” and this is in our opinion ethers comes short. We believe that extensibility such as support for custom transactions will simply help ethers to maintain its #1 spot. viem is a very good example of the utility of extensibility. Since the introduction of custom serialisers for Celo, there are now many other chains with custom extensions officially featured in the library. The OP stack extension for example furthers Ethereum’s L2 vision by providing a serializer for L2 deposit transactions In contrast to the experience in viem, providing support for other transaction types for ethers requires modifying several classes from ethers in a brittle way. For celo-ethers-wrapper we have found this is difficult to maintain, violates SOLID principles, and is a sore point for developers. And celo community developers have shown their desire for support for fee currencies in ethers already. How to make it happen? Based on the discussion (+ few more) there are multiple ways how to go forward:
or combination of some/all of the above. We’re more than happy to provide code snippets, create a branch with some ideas implemented, collaborate on making this happen, but first we’d love to hear your feedback and agree on the plan on how to make it happen. Thank you for your support for this initiative and engagement into the discussion so far! |
I General I think libraries for protocols should have easy ways to extend the protocol they support, usually by simply exposing the relevant fields as public and writable, seems like the easiest route. |
Hey @ricmoo, hope you’re well :) I’m Arthur 👋 @shazarre, @nicolasbrugneaux, @aaronmgdr, and I work together on all things developer tooling for Celo. USDC deployed on Celo yesterday, and is planning to become a “fee currency” in the next couple of weeks (pending a governance vote). ![]() Source: Circle - USDC Now Available on Celo We are very focused on the developer experience of using fee currencies on Celo (a highly Celo-specific feature that can be used via a custom typed transaction envelope defined here). Unfortunately, it’s not possible to conveniently serialize and sign this transaction type using We are anticipating a significant inflow of developers wanting to build with fee currencies on Celo, following the launch of USDC this week, and seeing that Opera has surpassed 1M+ users across Africa with its fee currency-enable wallet (MiniPay). ![]() Source: Twitter We are ok recommending that new developers use My colleague @shazarre quickly put together a PR (against our fork of Of course, this is not perfect and can be improved, but I hope this gives you a sense of how much we want to help and move things along. I mentioned this above, but for context, we were the first to work with Unfortunately, without a way forward to serialize and sign custom transaction type envelopes, we can’t recommend developers build with Is there anything we can do to help here? We wanted to be extra respectful, and have not opened a PR without your approval first. Would you be open to a draft PR we submit for this? We can always close the draft PR if this doesn't lead to a design you are happy with. We would just like to give this a chance. |
@ricmoo I pushed yesterday another demo of how the support could look like: celo-org#2, this one introduces more type-like approach (allowing for adding new top-level transaction fields while calling |
But this not supported by metamask which not accept customized Tx Type? |
Describe the Feature
I would like to be able to send and sign transactions which have types beyond null 1 and 2 and which may contain additional parameters.
Currently Ethers does not allow transactions with other types and actively removes any unknown object parameters making the use of alternative transaction types impossible.
My initial use for this will be using Celo's
0x7c
ie type124
which adds support the gasFeeCurrency parameter to transactions.//////
I plan to do the work for this. But want to start a discussion about how ethers would prefer to address this.
I could just submit a PR to allow the specific use case I have. Alternatively perhaps ethers could support some sort of bring your own Transaction Envelope parser. As of yet I don't know how it would look or if it would actually have benefits in code size and understandability.
Code Example
Other cases where alternative Transaction details are being requested
The text was updated successfully, but these errors were encountered: