SDK update 3.1.8 broke estimateL1GasCost for type 0 txs

[shanefontaine]

Describe the bug

The fix in #8902 explicitly defines values to be serialized by ethers.

For type 0 txs, ethers will accept the input and check the property keys. Since type 2 properties now exist on the tx object (even if undefined), they fail the check.

To Reproduce

Call estimateL1GasCost() with type 0 params on ethers 5.7.2.

Expected behavior

Return estimated l1 gas cost.

[roninjin10]

I see. We are on an old version of ethers so we can't really fix this issue with ethers directly. As a quick fix my suggestion is we use serializeTransaction from viem as a one off here. Viem should also be moved from devDeps to deps if it it isn't already a regular dep

I won't be able to get to this pr this week but happy to review a pr if you want to make this change in meantime

[shanefontaine]

Thanks for the reply @roninjin10

This is actually how I would expect ethers to behave. I don't think txs should have both type 0 and type 2 values present since then the intention of the caller would be unclear and it is left up to the lib to determine, which is not ideal. In this case, the consumer of the Optimism SDK has no power since both types are forced upon the call in all cases.

Is it possible for you to handle it prior to serialization with ethers? I'm not exactly sure where, but I would think ethers has a utility function somewhere that handles this.

[roninjin10]

Yes @shanefontaine. What you can do is filter out the undefined values. Viem will work without doing this because they check the value instead of the key. ('key' in value) vs (value.key !== undefined). Only downside to filtering out the keys is it's more code than using viem but upside is you aren't mixing viem and ethersv5.

[shanefontaine]

@roninjin10 sorry I might not be clear. I believe there is nothing I can do because the Optimism SDK is what adds the values no matter what I pass in (including undefined values). No matter what filtering I do, any type 0 tx will always fail because of this. Let me know if that helps!

[roninjin10]

@shanefontaine Right sorry I'm the one that is not clear. I'm talking about fixing the sdk not your repo. I worded it confusing because I said What you can do is which meant what one can do is or what whoever picks up this ticket can do is

[tynes]

The sdk is very fragile, it seems like every change that happens with it breaks something. There isn't good test coverage for it right now, the real main test for it is it runs both an eth and erc20 withdrawal against the docker compose devnet

[tynes]

The sdk is deprecated