fix: inferTypes error

[jasonandjay]

Specific errors and solutions from hre #4858

[yhuard]

Hi! I'd love to see this PR merged, as it would fix an issue where ethers v6 incorrectly infers the transaction type to be 1 instead of 0. In my case, my transaction contains a gasPrice and no accessList, therefore I think it should be considered as a legacy type (0) transaction. The transaction should only be considered as type 1 if it contains an accessList. This is the logic implemented by viem for instance.

@ricmoo is there any reason to not merge it?

The current logic looks flawed to me because oftypes.sort() which renders if (!hasAccessList) { types.push(0); } useless.

            } else if (hasGasPrice) {
                types.push(1);
                if (!hasAccessList) { types.push(0); }
            } else if (hasAccessList) {

...

        types.sort();       

Thanks in advance.

[ricmoo]

The inferring is based on what types of transaction if “could” be. In the event of ambiguity, it favours the most recent type that is still compatible.

I think this still makes sense, but am open to discussing it. But keep in mind this type of change could also break existing code which relies on the type being selected as type 1.

The `inferTypes' in this case is still “useful” (I don’t actually care much for inferred types and these reasons are exactly why), since it helps isolate what types it is valid for, for some applications that care about that. But the most recent fork is what should be targeted.

Is the issue that you are on a network that doesn’t support modern transaction types? It might make sense in a future update to include a comparator on the Network object to indicate the preferred transaction type (and allow rejecting invalid values).

This will become much more difficult as new transaction types aren’t necessary canonically ordered. :(

[yhuard]

Thank you for taking the time to reply.
Indeed, I'm working on a private network that only supports legacy transactions, so right now I have to explicitly set the type to 0 before signing the transaction. Not a huge deal for sure, but it's annoying if we have to explain to third parties that they also have to apply this trick if they want to use ethers v6 with our network.
For the same transaction, ethers.js v5 was inferring the type as type 0, and so would viem. ethers v6 is the only exception as far as I can tell (but I haven't tested all the libraries obviously)
I understand that there's a risk of breaking some implementations that use v6 and rely on the transaction to be inferred as type 1. However, does it make sense to send a type 1 transaction without an accessList? Isn't it the same thing as sending a type 0 transaction? I don't know if the EVM clients make a distinction between a type 0 transaction and a type 1 transaction without accessList. It's definitely something I'd have to look into.

[jxom]

But if it has an gasPrice and an accessList it cannot be type 0. If it has a gasPrice I think we still want to infer type 1. We want to infer the highest possible type from the provided parameters.

If gasPrice exists, but accessList is absent, it has to be of type 0. The accessList property is required on type 1 transactions. See here.

(sorry for my unsolicited input - just lurking the “Viem” keyword on GitHub search 😂)

[ricmoo]

Any unsolicited feedback is always welcome! :)

The issue though, is that properties implicit value. So, if the .value is unspecified it is 0, if the .data is unspecified it is 0x, otherwise for every transaction every field would need to be provided.

So, the transaction { to, data } would assume the .type is 2, and therefore "just work" as long as the network supported EIP-1559 transactions. It is also valid as a .type of 0, since it doesn't contain any conflicting fields. Things like .value are 0 and depending on the context .nonce may be implicitly set to 0 or populated by the network. The .inferTypes would include [ 0, 1, 2 ], since it is valid for any of them, but the inferred type would be 2, since that is the most recent fork.

This becomes an issue as we add more types, since just because a number is canonically larger, no longer means it should necessarily be the default type. And some exotic networks now have their own (much higher) transaction envelope types. So, going forward .inferTypes I don't think is practical. That's another discussion though.

On the one hand if we suggest people include .type in their transaction, they won't benefit from backwards compatible future forks (like EIP-1559, which required no changes to dapps and they instantly began benefitting from the lower transaction cost and more deterministic inclusion times) but it also means when new forks with potentially non-backwards-compatible types are introduced, their app may cease to operate. Akin to the "static" vs. "dynamic" linking issues for binaries.

This is why I'm thinking we need some sort of Network-level assistance, in determining the correct implicit .type.