Skip to content
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

Enable EIP-155 transactions as part of FIP-0091 #1003

Merged
merged 5 commits into from May 13, 2024

Conversation

aarshkshah1992
Copy link
Contributor

@aarshkshah1992 aarshkshah1992 commented May 8, 2024

This PR updates FIP-0091 to also enable support for EIP-155 transactions so that Coinbase Wallet , Trust Wallet etc can submit EIP-155 EVM transactions to Filecoin.

https://eips.ethereum.org/EIPS/eip-155

@aarshkshah1992 aarshkshah1992 changed the title [WIP] Enable EIP-155 transactions as part of FIP-0091 Enable EIP-155 transactions as part of FIP-0091 to enable Coinbase Wallet support (among other wallets) May 8, 2024
@aarshkshah1992 aarshkshah1992 changed the title Enable EIP-155 transactions as part of FIP-0091 to enable Coinbase Wallet support (among other wallets) Enable EIP-155 transactions as part of FIP-0091 May 8, 2024
Copy link

@TippyFlitsUK TippyFlitsUK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some very minor suggestions for flow.

FIPS/fip-0091.md Outdated Show resolved Hide resolved
FIPS/fip-0091.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@kaitlin-beegle kaitlin-beegle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial review complete- will need to finish in a few hours. However, a few clarification issues highlighted for the author.

FIPS/fip-0091.md Outdated Show resolved Hide resolved
FIPS/fip-0091.md Outdated Show resolved Hide resolved
@@ -9,12 +9,12 @@ category: Core
created: 2024-04-17
---

# FIP-00XX: Add support for legacy Homestead Ethereum Transactions
# FIP-0091: Add support for Homstead and EIP-155 Ethereum Transactions ("legacy" Ethereum transactions)

## Simple Summary
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarshkshah1992 just want to check before I make the change everywhere- I believe you are referencing EIP1155, not EIP155. I don't believe the latter exists. Yes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EIP155 exists and is correct (and linked in the original PR) -- it introduces chain_id as part of the tx.

EIP1155 doesn't exist, it's actually an ERC number.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaitlin-beegle Hey yes as Jorge pointed out above, EIP-155 can be found at https://eips.ethereum.org/EIPS/eip-155.

FIPS/fip-0091.md Outdated Show resolved Hide resolved
Comment on lines +62 to +64
To ensure compatibility with Filecoin's signature verification, which only recognizes `V` values of 0 or 1 (as is also the case with the current signature verification implementation in `go-ethereum`), adjustments will be made to the `V` parameter in legacy transactions before verifying their signatures. Specifically, for Homestead transactions, 27 will be subtracted from the `V` parameter. For EIP-155 transactions, the adjustment will involve subtracting (2*ChainID + 35) from the `V` parameter.

This adjustment is necessary because, unlike EIP-1559 transactions supported by Filecoin that strictly use `V` values of 0 or 1, the `V` parameter in Homestead transactions can be either 27 or 28, and for EIP-155 transactions, it is calculated as (0 or 1 + (2 * ChainID + 35)) where the selection of 0 or 1 is the same as in EIP-1599 transactions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To ensure compatibility with Filecoin's signature verification, which only recognizes `V` values of 0 or 1 (as is also the case with the current signature verification implementation in `go-ethereum`), adjustments will be made to the `V` parameter in legacy transactions before verifying their signatures. Specifically, for Homestead transactions, 27 will be subtracted from the `V` parameter. For EIP-155 transactions, the adjustment will involve subtracting (2*ChainID + 35) from the `V` parameter.
This adjustment is necessary because, unlike EIP-1559 transactions supported by Filecoin that strictly use `V` values of 0 or 1, the `V` parameter in Homestead transactions can be either 27 or 28, and for EIP-155 transactions, it is calculated as (0 or 1 + (2 * ChainID + 35)) where the selection of 0 or 1 is the same as in EIP-1599 transactions.
To ensure compatibility with Filecoin's signature verification, which only recognizes `V` values of 0 or 1 (as is also the case with the current signature verification implementation in `go-ethereum`), adjustments will be made to the `V` parameter in legacy transactions before verifying their signatures.
For Homestead transactions, the 'V' parameter will be calculated as _V_-27. This is because Homestead transactions are coded as either 27 or 28.
For EIP-155 transactions, the adjustment will involve subtracting (2*ChainID + 35) from the `V` parameter. This is because it is calculated as (0 or 1 + (2 * ChainID + 35)) where the selection of 0 or 1 is the same as in EIP-1599 transactions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original more descriptive language will help flush out better the change that needs to be made here.

Co-authored-by: Kaitlin Beegle <46908964+kaitlin-beegle@users.noreply.github.com>
Co-authored-by: TippyFlits <james@filoz.org>
Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial rubber stamp

@anorth anorth merged commit 1837481 into master May 13, 2024
1 check passed
@anorth anorth deleted the feat/enable-eip-155-transactions-fip-091 branch May 13, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants