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

l2geth: forward tx to sequencer #2011

Merged
merged 5 commits into from
Jan 19, 2022

Conversation

tuxcanfly
Copy link
Contributor

@tuxcanfly tuxcanfly commented Jan 14, 2022

Description

This PR makes it easier for replica nodes to handle a transaction submitted via RPC and to transparently forward it to the sequencer, based on the configured endpoint.

This allows the consumer of the replica node's RPC to directly submit transactions instead of having to rely on third-party applications.

Metadata

@changeset-bot
Copy link

changeset-bot bot commented Jan 14, 2022

⚠️ No Changeset found

Latest commit: 805a42c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added 2-reviewers A-cannon Area: cannon A-ops Area: ops labels Jan 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2022

Codecov Report

Merging #2011 (805a42c) into develop (3174f03) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #2011   +/-   ##
========================================
  Coverage    74.58%   74.58%           
========================================
  Files           79       79           
  Lines         2554     2554           
  Branches       401      401           
========================================
  Hits          1905     1905           
  Misses         649      649           
Flag Coverage Δ
batch-submitter 62.50% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.64% <ø> (ø)
message-relayer 70.86% <ø> (ø)
sdk 88.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3174f03...805a42c. Read the comment docs.

@github-actions github-actions bot added A-op-batcher Area: op-batcher A-integration Area: integration tests labels Jan 18, 2022
@tuxcanfly tuxcanfly marked this pull request as ready for review January 18, 2022 13:44
@github-actions github-actions bot removed A-op-batcher Area: op-batcher A-integration Area: integration tests labels Jan 18, 2022
@tynes
Copy link
Contributor

tynes commented Jan 18, 2022

This looks good to me, some thoughts:

  • We should have an integration test for this functionality, its really easy to add. This functionality should be preserved in the 1.0 release due to the public mempool being added back to the system. The logic would be similar to this:
    const signed = await env.l2Wallet.signTransaction(tx)
 const signed = await env.l2Wallet.signTransaction(tx)
 const result = await env.replicaProvider.sendTransaction(signed)

let receipt: TransactionReceipt
while (!receipt) {
  receipt = await env.replicaProvider.getTransactionReceipt(result.hash)
  await sleep(200)
}
  • Once l2geth: bring back unsupported RPC methods #2007, then the RPC methods that manage user keys should probably also be updated to proxy (eth_sendTransaction). I don't think that this is a super high priority as its unlikely many people will use it, but it would better match people's expectations on how upstream works.

Copy link
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

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

Good work! Will approve once the integration test is added :)

@github-actions github-actions bot added the A-integration Area: integration tests label Jan 19, 2022
@tuxcanfly
Copy link
Contributor Author

Thanks, added the integration test.

@tynes tynes merged commit 9d9bbc9 into ethereum-optimism:develop Jan 19, 2022
tynes added a commit that referenced this pull request Jan 19, 2022
Add missing changeset for #2011

A replica can be configured with the flag
`--sequencer.clienthttp` or the env var
`SEQUENCER_CLIENT_HTTP` which points to the
sequencer and when users hit the RPC endpoint
`eth_sendRawTransaction` it will forward the
value to the sequencer.

If running a mainnet node, this value should be
set to `https://mainnet.optimism.io` or an
infrastructure provider such as Quiknode, Infura
or Alchemy.
@github-actions github-actions bot mentioned this pull request Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cannon Area: cannon A-integration Area: integration tests A-ops Area: ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have replicas forward write requests to the Sequencer
4 participants