hyperlane-xyz / hyperlane-monorepo

The home for Hyperlane core contracts, sdk packages, and other infrastructure
https://hyperlane.xyz
Other
295 stars 318 forks source link

Legacy Ethereum chains aren't supported by our agent's use of ethers-rs #1976

Open tkporter opened 1 year ago

tkporter commented 1 year ago

In a debugging session with a PI deployer, we found an interesting issue. They were doing a PI deployment to two internal development chains so they can set up some integration tests, and these chains were ran using old Parity nodes from 2020.

The issue is that ethers-rs will by default add a type property to transactions to differentiate legacy transactions (these are type 0 or if type is not present), EIP 2930 transactions (this is when access lists were used, these are type 1), or EIP 1559 transactions (these are type 2). The Parity node is old and throws an error when RPCs provide transactions that include the type parameter. Note that simple eth_calls using ethers-rs would actually have this type as well, so the validator would make eth_call RPCs with the parameter "type":"0x00", and the Parity node would return errors.

You can see the offending type here: https://github.com/gakonst/ethers-rs/blob/master/ethers-core/src/types/transaction/eip2718.rs#L27-L40

Note that if ethers-rs is compiled with the feature legacy, the type parameter will not be present. As a temporary workaround, I created this branch https://github.com/hyperlane-xyz/hyperlane-monorepo/tree/trevor/legacy-ethers which uses the ethers-rs legacy feature. Unfortunately cast also doesn't work, even when using --legacy, because it uses ethers-rs under the hood and the --legacy flag just means that it won't try to use EIP 1559, not that it uses a version of ethers-rs built with the legacy feature. So I also provided https://github.com/tkporter/send-announce-tx to help announce using ethers-js instead of cast

How can we best provide agents with the legacy ethers-rs feature? Should we just document this, provide a diff, and make people build from source? Or is this common enough where we should build another Docker image? This issue tracks deciding how to deal with this & making any changes required

tkporter commented 1 year ago

Personally I think documenting this is sufficient -- for this PI deployer they actually have plans to move to a newer Ethereum node at some point too, and I think it's unlikely for many others to run into this given that EIP 2930 went live in 2021 and any nodes built in 2021 or later will support this

nambrot commented 1 year ago

I would suggest to keep this outside the bounds right now and hope there are not many chains like this out there