From d141095c48ea9b0a0d6813d64cb3ba42de97efae Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 8 Nov 2021 12:49:34 -0800 Subject: [PATCH 1/7] l2geth: allow for unprotected txs The old transaction batch serialization assumed that the correct chain id was used as there was a custom transaction serialization used. Now that the full RLP of the transactions are submitted to L1 for availability, it is possible to enable non EIP155 transactions. This allows for universal addresses to be used, where the same address exists across many chains. A test is updated to test specifically that transactions that are not protected with a chain id can be submitted and accepted by the sequencer. --- .changeset/old-moons-invite.md | 6 ++++++ integration-tests/test/rpc.spec.ts | 11 +++++++---- l2geth/internal/ethapi/api.go | 3 --- 3 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 .changeset/old-moons-invite.md diff --git a/.changeset/old-moons-invite.md b/.changeset/old-moons-invite.md new file mode 100644 index 0000000000000..897af9a3ff959 --- /dev/null +++ b/.changeset/old-moons-invite.md @@ -0,0 +1,6 @@ +--- +'@eth-optimism/integration-tests': patch +'@eth-optimism/l2geth': patch +--- + +Allow for unprotected transactions diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index afaadd2a30750..240e87821c527 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/integration-tests/test/rpc.spec.ts @@ -84,16 +84,19 @@ describe('Basic RPC tests', () => { ).to.be.rejectedWith('invalid transaction: invalid sender') }) - it('should not accept a transaction without a chain ID', async () => { + it('should accept a transaction without a chain ID', async () => { const tx = { ...defaultTransactionFactory(), + nonce: await wallet.getTransactionCount(), gasPrice: await gasPriceForL2(env), chainId: null, // Disables EIP155 transaction signing. } + const signed = await wallet.signTransaction(tx) + const response = await provider.sendTransaction(signed) - await expect( - provider.sendTransaction(await wallet.signTransaction(tx)) - ).to.be.rejectedWith('Cannot submit unprotected transaction') + expect(response.chainId).to.equal(0) + const v = response.v + expect(v === 27 || v === 28).to.be.true }) it('should accept a transaction with a value', async () => { diff --git a/l2geth/internal/ethapi/api.go b/l2geth/internal/ethapi/api.go index 198e259259530..7b6f1a27852df 100644 --- a/l2geth/internal/ethapi/api.go +++ b/l2geth/internal/ethapi/api.go @@ -1573,9 +1573,6 @@ func (args *SendTxArgs) toTransaction() *types.Transaction { // SubmitTransaction is a helper function that submits tx to txPool and logs a message. func SubmitTransaction(ctx context.Context, b Backend, tx *types.Transaction) (common.Hash, error) { - if !tx.Protected() { - return common.Hash{}, errors.New("Cannot submit unprotected transaction") - } if err := b.SendTx(ctx, tx); err != nil { return common.Hash{}, err } From 128e0f84629d0302a9688cd427f3f02be4bd689c Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 8 Nov 2021 13:10:53 -0800 Subject: [PATCH 2/7] dtl: support unprotected txs When syncing from L2, all transactions were assumed to be protected. To prevent a network split for replicas, the DTL needs to check for transactions that are not protected and handle parsing the `v` parameter appropriately. --- .changeset/nice-steaks-hammer.md | 5 +++++ packages/data-transport-layer/src/utils/eth-tx.ts | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 .changeset/nice-steaks-hammer.md diff --git a/.changeset/nice-steaks-hammer.md b/.changeset/nice-steaks-hammer.md new file mode 100644 index 0000000000000..3e5cb7f8f8b36 --- /dev/null +++ b/.changeset/nice-steaks-hammer.md @@ -0,0 +1,5 @@ +--- +'@eth-optimism/data-transport-layer': patch +--- + +Handle unprotected transactions diff --git a/packages/data-transport-layer/src/utils/eth-tx.ts b/packages/data-transport-layer/src/utils/eth-tx.ts index cce210d269098..f20d9faa821ab 100644 --- a/packages/data-transport-layer/src/utils/eth-tx.ts +++ b/packages/data-transport-layer/src/utils/eth-tx.ts @@ -2,8 +2,18 @@ import { ethers } from 'ethers' export const parseSignatureVParam = ( - v: number | ethers.BigNumber, + v: number | ethers.BigNumber | string, chainId: number ): number => { - return ethers.BigNumber.from(v).toNumber() - 2 * chainId - 35 + v = ethers.BigNumber.from(v).toNumber() + // Handle normalized v + if (v === 0 || v === 1) { + return v + } + // Handle unprotected transactions + if (v === 27 || v === 28) { + return v - 27 + } + // Handle EIP155 transactions + return v - 2 * chainId - 35 } From 2745279026c63ef79f6cf8aa65639855a088615c Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 8 Nov 2021 14:51:46 -0800 Subject: [PATCH 3/7] tests: add replica tests --- integration-tests/test/replica.spec.ts | 64 ++++++++++++++++++++++++++ integration-tests/test/shared/env.ts | 4 ++ integration-tests/test/shared/utils.ts | 12 +++-- ops/docker-compose.yml | 6 +-- 4 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 integration-tests/test/replica.spec.ts diff --git a/integration-tests/test/replica.spec.ts b/integration-tests/test/replica.spec.ts new file mode 100644 index 0000000000000..c8b744692fdc5 --- /dev/null +++ b/integration-tests/test/replica.spec.ts @@ -0,0 +1,64 @@ +import { OptimismEnv } from './shared/env' +import { defaultTransactionFactory, gasPriceForL2, sleep } from './shared/utils' +import { expect } from 'chai' + +describe('Replica Tests', () => { + let env: OptimismEnv + + before(async () => { + env = await OptimismEnv.new() + }) + + describe('Matching blocks', () => { + it('should sync a transaction', async () => { + const tx = defaultTransactionFactory() + tx.gasPrice = await gasPriceForL2() + const result = await env.l2Wallet.sendTransaction(tx) + + let receipt + while (!receipt) { + receipt = await env.replicaProvider.getTransactionReceipt(result.hash) + await sleep(200) + } + + const sequencerBlock = (await env.l2Provider.getBlock( + result.blockNumber + )) as any + + const replicaBlock = (await env.replicaProvider.getBlock( + result.blockNumber + )) as any + + expect(sequencerBlock.stateRoot).to.deep.eq(replicaBlock.stateRoot) + expect(sequencerBlock.hash).to.deep.eq(replicaBlock.hash) + }) + + it('sync an unprotected tx (eip155)', async () => { + const tx = { + ...defaultTransactionFactory(), + nonce: await env.l2Wallet.getTransactionCount(), + gasPrice: await gasPriceForL2(), + chainId: null, // Disables EIP155 transaction signing. + } + const signed = await env.l2Wallet.signTransaction(tx) + const result = await env.l2Provider.sendTransaction(signed) + + let receipt + while (!receipt) { + receipt = await env.replicaProvider.getTransactionReceipt(result.hash) + await sleep(200) + } + + const sequencerBlock = (await env.l2Provider.getBlock( + result.blockNumber + )) as any + + const replicaBlock = (await env.replicaProvider.getBlock( + result.blockNumber + )) as any + + expect(sequencerBlock.stateRoot).to.deep.eq(replicaBlock.stateRoot) + expect(sequencerBlock.hash).to.deep.eq(replicaBlock.hash) + }) + }) +}) diff --git a/integration-tests/test/shared/env.ts b/integration-tests/test/shared/env.ts index 2e2120397d3e0..bdfd02b3d0b12 100644 --- a/integration-tests/test/shared/env.ts +++ b/integration-tests/test/shared/env.ts @@ -10,6 +10,7 @@ import { getAddressManager, l1Provider, l2Provider, + replicaProvider, l1Wallet, l2Wallet, fundUser, @@ -52,6 +53,7 @@ export class OptimismEnv { // The providers l1Provider: providers.JsonRpcProvider l2Provider: providers.JsonRpcProvider + replicaProvider: providers.JsonRpcProvider constructor(args: any) { this.addressManager = args.addressManager @@ -67,6 +69,7 @@ export class OptimismEnv { this.l2Wallet = args.l2Wallet this.l1Provider = args.l1Provider this.l2Provider = args.l2Provider + this.replicaProvider = args.replicaProvider this.ctc = args.ctc this.scc = args.scc } @@ -126,6 +129,7 @@ export class OptimismEnv { l2Wallet, l1Provider, l2Provider, + replicaProvider, }) } diff --git a/integration-tests/test/shared/utils.ts b/integration-tests/test/shared/utils.ts index eef601c4cb3b1..82f1f68e3068c 100644 --- a/integration-tests/test/shared/utils.ts +++ b/integration-tests/test/shared/utils.ts @@ -55,13 +55,19 @@ const env = cleanEnv(process.env, { export const l1Provider = new providers.JsonRpcProvider(env.L1_URL) l1Provider.pollingInterval = env.L1_POLLING_INTERVAL -export const l2Provider = new providers.JsonRpcProvider(env.L2_URL) +export const l2Provider = injectL2Context( + new providers.JsonRpcProvider(env.L2_URL) +) l2Provider.pollingInterval = env.L2_POLLING_INTERVAL -export const verifierProvider = new providers.JsonRpcProvider(env.VERIFIER_URL) +export const verifierProvider = injectL2Context( + new providers.JsonRpcProvider(env.VERIFIER_URL) +) verifierProvider.pollingInterval = env.VERIFIER_POLLING_INTERVAL -export const replicaProvider = new providers.JsonRpcProvider(env.REPLICA_URL) +export const replicaProvider = injectL2Context( + new providers.JsonRpcProvider(env.REPLICA_URL) +) replicaProvider.pollingInterval = env.REPLICA_POLLING_INTERVAL // The sequencer private key which is funded on L1 diff --git a/ops/docker-compose.yml b/ops/docker-compose.yml index ed6c69de460f6..b7b70dc64481d 100644 --- a/ops/docker-compose.yml +++ b/ops/docker-compose.yml @@ -165,7 +165,7 @@ services: depends_on: - dtl deploy: - replicas: 0 + replicas: 1 build: context: .. dockerfile: ./ops/docker/Dockerfile.geth @@ -181,8 +181,8 @@ services: ETH1_CTC_DEPLOYMENT_HEIGHT: 8 RETRIES: 60 ports: - - ${L2GETH_HTTP_PORT:-8549}:8545 - - ${L2GETH_WS_PORT:-8550}:8546 + - ${REPLICA_HTTP_PORT:-8549}:8545 + - ${REPLICA_WS_PORT:-8550}:8546 integration_tests: deploy: From ec1bcd48bdf93ed148ccef2b77b5dc5a89bbc448 Mon Sep 17 00:00:00 2001 From: kf Date: Thu, 18 Nov 2021 20:14:40 +0000 Subject: [PATCH 4/7] fix: update rollup client for unprotected tx support --- integration-tests/package.json | 1 + integration-tests/test/replica.spec.ts | 9 ++--- l2geth/rollup/client.go | 33 +++++++++++-------- .../data-transport-layer/src/utils/eth-tx.ts | 6 +--- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/integration-tests/package.json b/integration-tests/package.json index f0bed14fa2a11..584cee059a6cc 100644 --- a/integration-tests/package.json +++ b/integration-tests/package.json @@ -31,6 +31,7 @@ "@eth-optimism/contracts": "0.5.4", "@eth-optimism/core-utils": "0.7.2", "@eth-optimism/message-relayer": "0.2.4", + "@ethersproject/abstract-provider": "^5.5.1", "@ethersproject/providers": "^5.4.5", "@ethersproject/transactions": "^5.4.0", "@nomiclabs/hardhat-ethers": "^2.0.2", diff --git a/integration-tests/test/replica.spec.ts b/integration-tests/test/replica.spec.ts index c8b744692fdc5..d08dba2b9be65 100644 --- a/integration-tests/test/replica.spec.ts +++ b/integration-tests/test/replica.spec.ts @@ -1,6 +1,7 @@ import { OptimismEnv } from './shared/env' import { defaultTransactionFactory, gasPriceForL2, sleep } from './shared/utils' import { expect } from 'chai' +import { TransactionReceipt } from '@ethersproject/abstract-provider' describe('Replica Tests', () => { let env: OptimismEnv @@ -12,10 +13,10 @@ describe('Replica Tests', () => { describe('Matching blocks', () => { it('should sync a transaction', async () => { const tx = defaultTransactionFactory() - tx.gasPrice = await gasPriceForL2() + tx.gasPrice = await gasPriceForL2(env) const result = await env.l2Wallet.sendTransaction(tx) - let receipt + let receipt: TransactionReceipt while (!receipt) { receipt = await env.replicaProvider.getTransactionReceipt(result.hash) await sleep(200) @@ -37,13 +38,13 @@ describe('Replica Tests', () => { const tx = { ...defaultTransactionFactory(), nonce: await env.l2Wallet.getTransactionCount(), - gasPrice: await gasPriceForL2(), + gasPrice: await gasPriceForL2(env), chainId: null, // Disables EIP155 transaction signing. } const signed = await env.l2Wallet.signTransaction(tx) const result = await env.l2Provider.sendTransaction(signed) - let receipt + let receipt: TransactionReceipt while (!receipt) { receipt = await env.replicaProvider.getTransactionReceipt(result.hash) await sleep(200) diff --git a/l2geth/rollup/client.go b/l2geth/rollup/client.go index 7d6f046061f73..5370fbc209427 100644 --- a/l2geth/rollup/client.go +++ b/l2geth/rollup/client.go @@ -134,8 +134,8 @@ type RollupClient interface { // Client is an HTTP based RollupClient type Client struct { - client *resty.Client - signer *types.EIP155Signer + client *resty.Client + chainID *big.Int } // TransactionResponse represents the response from the remote server when @@ -166,11 +166,10 @@ func NewClient(url string, chainID *big.Int) *Client { } return nil }) - signer := types.NewEIP155Signer(chainID) return &Client{ - client: client, - signer: &signer, + client: client, + chainID: chainID, } } @@ -322,7 +321,7 @@ func (c *Client) GetLatestTransactionBatchIndex() (*uint64, error) { // batchedTransactionToTransaction converts a transaction into a // types.Transaction that can be consumed by the SyncService -func batchedTransactionToTransaction(res *transaction, signer *types.EIP155Signer) (*types.Transaction, error) { +func batchedTransactionToTransaction(res *transaction, chainID *big.Int) (*types.Transaction, error) { // `nil` transactions are not found if res == nil { return nil, errElementNotFound @@ -373,7 +372,15 @@ func batchedTransactionToTransaction(res *transaction, signer *types.EIP155Signe sig := make([]byte, crypto.SignatureLength) copy(sig[32-len(r):32], r) copy(sig[64-len(s):64], s) - sig[64] = byte(res.Decoded.Signature.V) + + var signer types.Signer + if res.Decoded.Signature.V == 27 || res.Decoded.Signature.V == 28 { + signer = types.HomesteadSigner{} + sig[64] = byte(res.Decoded.Signature.V - 27) + } else { + signer = types.NewEIP155Signer(chainID) + sig[64] = byte(res.Decoded.Signature.V) + } tx, err := tx.WithSignature(signer, sig[:]) if err != nil { @@ -431,7 +438,7 @@ func (c *Client) GetTransaction(index uint64, backend Backend) (*types.Transacti if !ok { return nil, fmt.Errorf("could not get tx with index %d", index) } - return batchedTransactionToTransaction(res.Transaction, c.signer) + return batchedTransactionToTransaction(res.Transaction, c.chainID) } // GetLatestTransaction will get the latest transaction, meaning the transaction @@ -452,7 +459,7 @@ func (c *Client) GetLatestTransaction(backend Backend) (*types.Transaction, erro return nil, errors.New("Cannot get latest transaction") } - return batchedTransactionToTransaction(res.Transaction, c.signer) + return batchedTransactionToTransaction(res.Transaction, c.chainID) } // GetEthContext will return the EthContext by block number @@ -564,7 +571,7 @@ func (c *Client) GetLatestTransactionBatch() (*Batch, []*types.Transaction, erro if !ok { return nil, nil, fmt.Errorf("Cannot parse transaction batch response") } - return parseTransactionBatchResponse(txBatch, c.signer) + return parseTransactionBatchResponse(txBatch, c.chainID) } // GetTransactionBatch will return the transaction batch by batch index @@ -584,19 +591,19 @@ func (c *Client) GetTransactionBatch(index uint64) (*Batch, []*types.Transaction if !ok { return nil, nil, fmt.Errorf("Cannot parse transaction batch response") } - return parseTransactionBatchResponse(txBatch, c.signer) + return parseTransactionBatchResponse(txBatch, c.chainID) } // parseTransactionBatchResponse will turn a TransactionBatchResponse into a // Batch and its corresponding types.Transactions -func parseTransactionBatchResponse(txBatch *TransactionBatchResponse, signer *types.EIP155Signer) (*Batch, []*types.Transaction, error) { +func parseTransactionBatchResponse(txBatch *TransactionBatchResponse, chainID *big.Int) (*Batch, []*types.Transaction, error) { if txBatch == nil || txBatch.Batch == nil { return nil, nil, errElementNotFound } batch := txBatch.Batch txs := make([]*types.Transaction, len(txBatch.Transactions)) for i, tx := range txBatch.Transactions { - transaction, err := batchedTransactionToTransaction(tx, signer) + transaction, err := batchedTransactionToTransaction(tx, chainID) if err != nil { return nil, nil, fmt.Errorf("Cannot parse transaction batch: %w", err) } diff --git a/packages/data-transport-layer/src/utils/eth-tx.ts b/packages/data-transport-layer/src/utils/eth-tx.ts index f20d9faa821ab..025b5d19ddcb0 100644 --- a/packages/data-transport-layer/src/utils/eth-tx.ts +++ b/packages/data-transport-layer/src/utils/eth-tx.ts @@ -6,13 +6,9 @@ export const parseSignatureVParam = ( chainId: number ): number => { v = ethers.BigNumber.from(v).toNumber() - // Handle normalized v - if (v === 0 || v === 1) { - return v - } // Handle unprotected transactions if (v === 27 || v === 28) { - return v - 27 + return v } // Handle EIP155 transactions return v - 2 * chainId - 35 From 300683c7cbf059e7b88f8e1d9d860e9166418b34 Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 29 Nov 2021 14:04:21 -0800 Subject: [PATCH 5/7] ci: collect l2geth and replica logs --- .github/workflows/integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 9803ac8bcb051..6ce8310aa82e5 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -55,7 +55,7 @@ jobs: if: failure() uses: jwalton/gh-docker-logs@v1 with: - images: 'ethereumoptimism/hardhat,ops_deployer,ops_dtl,ethereumoptimism/l2geth,ethereumoptimism/message-relayer,ops_batch_submitter,ethereumoptimism/l2geth,ops_integration_tests' + images: 'ethereumoptimism/hardhat,ops_deployer,ops_dtl,ops_l2geth,ethereumoptimism/message-relayer,ops_batch_submitter,ops_replica,ops_integration_tests' dest: '/home/runner/logs' - name: Tar logs From 50feea32a0b62da6daed53185db1ccd4f833848a Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 29 Nov 2021 14:37:06 -0800 Subject: [PATCH 6/7] ops: pass through replica and verifier urls --- ops/docker-compose.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ops/docker-compose.yml b/ops/docker-compose.yml index b7b70dc64481d..d019630cbe4bb 100644 --- a/ops/docker-compose.yml +++ b/ops/docker-compose.yml @@ -195,6 +195,8 @@ services: environment: L1_URL: http://l1_chain:8545 L2_URL: http://l2geth:8545 + REPLICA_URL: http://replica:8545 + VERIFIER_URL: http://verifier:8545 URL: http://deployer:8081/addresses.json ENABLE_GAS_REPORT: 1 NO_NETWORK: 1 From 6bd6768e3d0c78238e69ac1c7e7f5617bd79d75b Mon Sep 17 00:00:00 2001 From: Mark Tyneway Date: Mon, 29 Nov 2021 15:05:08 -0800 Subject: [PATCH 7/7] integration-tests: skip replica tests on live network --- integration-tests/test/replica.spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/integration-tests/test/replica.spec.ts b/integration-tests/test/replica.spec.ts index d08dba2b9be65..b4ea35b8b4a0f 100644 --- a/integration-tests/test/replica.spec.ts +++ b/integration-tests/test/replica.spec.ts @@ -1,5 +1,10 @@ import { OptimismEnv } from './shared/env' -import { defaultTransactionFactory, gasPriceForL2, sleep } from './shared/utils' +import { + defaultTransactionFactory, + gasPriceForL2, + sleep, + isLiveNetwork, +} from './shared/utils' import { expect } from 'chai' import { TransactionReceipt } from '@ethersproject/abstract-provider' @@ -11,6 +16,11 @@ describe('Replica Tests', () => { }) describe('Matching blocks', () => { + if (isLiveNetwork()) { + console.log('Skipping replica tests on live network') + return + } + it('should sync a transaction', async () => { const tx = defaultTransactionFactory() tx.gasPrice = await gasPriceForL2(env)