feat: ETH compatibility in Filecoin : Support legacy Homestead Ethereum transactions and legacy EIP-155 transactions in Filecoin #11930
Conversation
aarshkshah1992
changed the title
aarshkshah1992
changed the title
aarshkshah1992
changed the title
|
Should we just make all ETH RPC APIs return all three of Users can always detect that a transaction is a legacy transaction if the |
This sounds like a good approach to me. |
| return nil, fmt.Errorf("signature should be %d bytes long, but got %d bytes", legacyHomesteadTxSignatureLen, len(sig)) | ||
| } | ||
| if sig[0] != LegacyHomesteadEthTxSignaturePrefix { | ||
| return nil, fmt.Errorf("expected signature prefix 0x01, but got 0x%x", sig[0]) |
There was a problem hiding this comment.
nit: 0x01 => 0x%x and add LegacyHomesteadEthTxSignaturePrefix as argument
|
|
||
| func (tx *EthLegacyHomesteadTxArgs) ToVerifiableSignature(sig []byte) ([]byte, error) { | ||
| if len(sig) != legacyHomesteadTxSignatureLen { | ||
| return nil, fmt.Errorf("signature should be %d bytes long, but got %d bytes", legacyHomesteadTxSignatureLen, len(sig)) |
There was a problem hiding this comment.
Not sure but might be instructive to add a little more information to avoid confusion with the literal length of eth homestead signatures being 1 byte less.
| return nil, fmt.Errorf("signature should be %d bytes long, but got %d bytes", legacyHomesteadTxSignatureLen, len(sig)) | |
| return nil, fmt.Errorf("signature should be %d bytes long (1 byte metadata, %d bytes sig data), but got %d bytes", legacyHomesteadTxSignatureLen, legacyHomesteadTxSignatureLen - 1, len(sig)) |
| require.NoError(t, err) | ||
| } | ||
|
|
||
| func prepareTxTestcases() ([]TxTestcase, error) { |
There was a problem hiding this comment.
any idea where these came from and if it would be easy to compile a similar list for legacy?
There was a problem hiding this comment.
@rvagg I am not sure where this came from. Maybe they pulled these up from etherscan. But it's a bit hard and time consuming to put these together for legacy txns as etherscan does not have a "filter by date" functionality. I was able to find some which I've used here.
I am also going to test this on a butterfly node using legacy txns generated by Metamask so we should be fine.
|
This looks pretty good, as far as my knowledge of such things go. It would be nice to add a corpus of legacy transactions to decode for the tests like the existing eip1559 ones, but if we don't have a ready source of that I don't think spending time collecting them is going to add enough value for the effort. It'd also be nice to have a label that we could mark PRs like this as "API breaking" to make sure we don't slip it into a semver-patch. |
|
@raulk @fridrik01 could you please help take a quick look on this PR by any chance? |
|
@rvagg @ZenGround0 Have addressed all your review comments so far. Testing this on a local node now. |
|
Tested and works with Metamask Legacy Transactions 
|
|
Works with Coinbase Wallet as well 
|
aarshkshah1992
changed the title
|
We need EIP-155 transactions to enable support for Glif, Coinbase Wallet, Phantom Wallet etc with EVM based Filecoin transactions as they use EIP-155 transactions under the hood. This is a big unlock for the ecosystem: The code difference with Homestead transactions entails two things: b) There is a difference in how we construct the digest for signature verification for EIP-155 transactions namely that we include the ChainID in the digest but not in the serialised transaction that is broadcast among nodes (as the digest can be reconstructed by extracting the ChainID from the V value of the signature) |
|
Broke this down into two PRs.
We will merge both of these to an integration and then merge the integration branch to master. |
Related Issues
Closes #11859
Proposed Changes
MaxFeePerGasandMaxPriorityFeePerGasChainIDwill be 0x00TxTypewill be 0x00GasPriceAdditional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps