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

feat(evm): Raw Ethereum transaction relay support #3773

Merged
merged 11 commits into from
Jan 13, 2021
Merged

Conversation

ilblackdragon
Copy link
Member

@ilblackdragon ilblackdragon commented Jan 2, 2021

Adds support for relaying signed transactions from Ethereum.
Can be used by MetaMask configured to the right RPC proxy or NEAR's Web3 provider in sendRawTransaction method.

Addresses few collateral issues:

  • converting chain_id to u64, as prior it was u128 which didn't match OE EVM interface
  • fixing view_call function when it's used for contract deployment (this is used for dry_run)

Test plan

Adding unit test for sending signed transactions: transfer, deploy code and function call. Testing invalid signature and chain_id passes inside the transaction.
Adding support for sendRawTransaction in near-web3-provider repo: aurora-is-near/near-web3-provider#70

@ilblackdragon ilblackdragon requested a review from artob January 2, 2021 12:49
@artob artob self-assigned this Jan 3, 2021
@artob artob added A-EVM Area: Native EVM implementation and support C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 3, 2021
Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

what's the reason for changing the type of chain_id from u128 to u64?

@ilblackdragon
Copy link
Member Author

@bowenwang1996 because chain_id is u64: https://github.com/near/nearcore/blob/master/runtime/near-evm-runner/src/near_ext.rs#L74
It was my original mistake to make it u128 everywhere in our code.

@ilblackdragon ilblackdragon changed the title feat: [EVM] Raw Ethereum transaction relay support [WIP] feat: [EVM] Raw Ethereum transaction relay support Jan 5, 2021
@artob
Copy link
Contributor

artob commented Jan 5, 2021

@bowenwang1996 because chain_id is u64: https://github.com/near/nearcore/blob/master/runtime/near-evm-runner/src/near_ext.rs#L74
It was my original mistake to make it u128 everywhere in our code.

To be pedantic, "Chain ID is a 256-bit value" per EIP-1344. However, in terms of OpenEthereum's APIs, they do only take a 64-bit value. And all registered chain IDs do fit within 64 bits, and certainly our own chain IDs do, so probably no good reason to need it be larger.

Copy link
Contributor

@artob artob left a comment

Choose a reason for hiding this comment

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

Looks really good overall, and significantly increases our out-of-the-box Eth1 compatibility.

My only comments are some nits that can be addressed in a later iteration.

@artob artob requested a review from bowenwang1996 January 5, 2021 22:23
Base automatically changed from evm-phase2 to master January 6, 2021 17:43
@ilblackdragon ilblackdragon changed the title [WIP] feat: [EVM] Raw Ethereum transaction relay support [WIP] feat(evm): Raw Ethereum transaction relay support Jan 7, 2021
…ransactions that will get executed on behalf of the sender
@artob artob force-pushed the evm-raw-tx-relay branch from f30839a to b3f4a09 Compare January 7, 2021 22:40
@artob artob requested review from ailisp and artob January 7, 2021 22:51
…alid and invalid sigs. Adding proper errors for invalid sigs and chain_id
@artob artob force-pushed the evm-raw-tx-relay branch from b3f4a09 to e66ca4a Compare January 7, 2021 23:22
@artob
Copy link
Contributor

artob commented Jan 7, 2021

@ilblackdragon @ailisp FYI, I've rebased this pull request against the current master branch just now.

Copy link
Collaborator

@evgenykuzyakov evgenykuzyakov left a comment

Choose a reason for hiding this comment

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

You need to bump EVM protocol version to hardfork betanet.

@artob
Copy link
Contributor

artob commented Jan 8, 2021

@bowenwang1996 Need your review as code owner to be able to merge this.

@ilblackdragon ilblackdragon changed the title [WIP] feat(evm): Raw Ethereum transaction relay support feat(evm): Raw Ethereum transaction relay support Jan 12, 2021
@artob
Copy link
Contributor

artob commented Jan 12, 2021

You need to bump EVM protocol version to hardfork betanet.

Done, just pushed now. @bowenwang1996 This is all good to go as far as @ilblackdragon and I are concerned, if you would have a chance to review.

@artob
Copy link
Contributor

artob commented Jan 12, 2021

@olonho @willemneal Could we get a review from you guys as well, pretty please, to unblock merging this?

Copy link
Contributor

@willemneal willemneal left a comment

Choose a reason for hiding this comment

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

LGTM just one logic question.

@artob artob merged commit e3cd9fc into master Jan 13, 2021
@artob artob deleted the evm-raw-tx-relay branch January 13, 2021 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-EVM Area: Native EVM implementation and support C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants