Remove duplicate Solidity sig check in fullnode proxy by karlfloersch · Pull Request #23 · ethereum-optimism/optimism

karlfloersch · 2020-02-25T04:47:47Z

Description

This PR speeds up tests by no longer using the fully Solidity validated EOA function call. Instead we just recover the tx contents once inside of the fullnode, and then make an authenticated call on that user's behalf.

It also fixes a really tricky bug which was causing our fullnode to crash inside of a try{}catch{} statement!!. Apparently you cannot use Ganache's call with the ZERO_ADDRESS if the call fails because that will cause a really nasty error:

/Users/karlfloersch/workspace/ethereum/optimism-monorepo/node_modules/ethereumjs-util/dist/index.js:369
  var sig = secp256k1.sign(msgHash, privateKey);
                      ^
Error: nonce generation function failed or private key is invalid
    at Object.exports.ecsign (/Users/karlfloersch/workspace/ethereum/optimism-monorepo/node_modules/ethereumjs-util/dist/index.js:369:23)
    at Transaction.sign (/Users/karlfloersch/workspace/ethereum/optimism-monorepo/node_modules/ethereumjs-tx/es5/index.js:252:23)

By making this small change: b4060c8 it fixed the issue. Something that's a little weirder is that doing something as simple as changing it to use 0x 0x0000000000000000000000000000000000000099 as the from field also solves the issue! So it's just something special about the ZERO_ADDRESS it seems...

TODO

It is probably best to verify that the nonce is correct. This should be a quick fix.

Metadata

Fixes

Fixes # YAS-194

Contributing Agreement

I have read and understood the Optimism Contributing Guide and Code of Conduct and am following those guidelines in this pull request.

willmeister

👍

willmeister · 2020-02-26T15:35:09Z

packages/rollup-full-node/src/app/handler.ts

👍

Would change to getCallData(...) and getTransactionData(...) to better indicate how these two are used in the grand scheme of things.

…itySigVerify

K-Ho · 2020-03-19T15:21:09Z

packages/ovm/src/contracts/ExecutionManager.sol

@@ -504,12 +502,13 @@ contract ExecutionManager is FullStateManager {
     * @param _timestamp The timestamp which should be used for this context.
     * @param _queueOrigin The queue which this context's transaction was sent from.


Add an @param comment for _ovmTxOrigin

K-Ho · 2020-03-19T15:24:13Z

packages/ovm/test/helpers.ts

  executionManager: Contract,
  wallet: Wallet,
  to: Address,
-  data: string
+  data: string,
+  allowRevert: boolean
 ): Promise<TransactionReceipt> => {


A function comment for executeUnsignedEOACall describing what each of the parameters is would be helpful! (Specifically about allowRevert)

K-Ho · 2020-03-19T15:26:45Z

packages/rollup-full-node/src/app/web3-rpc-handler.ts

  }

-  private generateUnsignedCallCalldata(
+  private getTransactionCalldata(


A short function comment would be helpful!

feat: update swaps

* feat: add NewVersion util * fix: set big pi store radix and leaf * refactor: generalize pi registry test

…reum-optimism#23)

* test: add L2 standard bridge interop unit tests (#13) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: unit tests fixes * fix: super to legacy tests failing * fix: mock and expect mint and burn * fix: add generic factory interface (#14) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: add generic factory interface * feat: modify OptimismMintableERC20Factory for convert (#17) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: add generic factory interface * feat: modify OptimismMintableERC20Factory for convert * fix: use only a public function for create3 * feat: rollback interop factory, modify legacy one * fix: delete local token return variable * fix: PR fixes * feat: add superchain erc20 factory implementation (#23) * feat: add superchain erc20 factory implementation * fix: remove createX comments * test: add superchain erc20 factory tests (#25) * test: add superchain erc20 factory tests * test: add erc20 asserts * test: fix expect emit * fix: remove comments * feat: add constructor to superchain ERC20 beacon (#34) * test: remove factory predeploy etch ---------- Co-authored-by: 0xng <ng@defi.sucks> Co-authored-by: 0xParticle <particle@defi.sucks> Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> * fix: set an arbitrary address for superchain erc20 impl * fix: deploy a proxy for the beacon on genesis (#45) --------- Co-authored-by: 0xng <ng@defi.sucks> * fix: conflicts and imports * fix: interfaces * chore: add .testdata * fix: adding back .testdata to gitignore * fix: new conflicts from ci improvements --------- Co-authored-by: 0xng <ng@defi.sucks> Co-authored-by: 0xParticle <particle@defi.sucks> Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com>

* test: add L2 standard bridge interop unit tests (ethereum-optimism#13) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: unit tests fixes * fix: super to legacy tests failing * fix: mock and expect mint and burn * fix: add generic factory interface (ethereum-optimism#14) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: add generic factory interface * feat: modify OptimismMintableERC20Factory for convert (ethereum-optimism#17) * test: add L2 standard bridge interop unit tests * fix: add tests natspec * fix: add generic factory interface * feat: modify OptimismMintableERC20Factory for convert * fix: use only a public function for create3 * feat: rollback interop factory, modify legacy one * fix: delete local token return variable * fix: PR fixes * feat: add superchain erc20 factory implementation (ethereum-optimism#23) * feat: add superchain erc20 factory implementation * fix: remove createX comments * test: add superchain erc20 factory tests (ethereum-optimism#25) * test: add superchain erc20 factory tests * test: add erc20 asserts * test: fix expect emit * fix: remove comments * feat: add constructor to superchain ERC20 beacon (ethereum-optimism#34) * test: remove factory predeploy etch ---------- Co-authored-by: 0xng <ng@defi.sucks> Co-authored-by: 0xParticle <particle@defi.sucks> Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> * fix: set an arbitrary address for superchain erc20 impl * fix: deploy a proxy for the beacon on genesis (ethereum-optimism#45) --------- Co-authored-by: 0xng <ng@defi.sucks> * fix: conflicts and imports * fix: interfaces * chore: add .testdata * fix: adding back .testdata to gitignore * fix: new conflicts from ci improvements --------- Co-authored-by: 0xng <ng@defi.sucks> Co-authored-by: 0xParticle <particle@defi.sucks> Co-authored-by: gotzenx <78360669+gotzenx@users.noreply.github.com> Co-authored-by: Disco <131301107+0xDiscotech@users.noreply.github.com>

README for pbs enabled devnet

* WIP: initial commit of e2e files from old repo * WIP: wip commit before moving e2e tests to op-geth * wip: commented out old tests * wip: change init file, docker compose and remove irrelevant tests * chore: add files to gitignore * fix: remove unnecessary cache pruning to optimise workflow * chore: remove e2e code that is now ported to op-geth * fix: pull branch to tmp directory & retry for leader detection * fix: echo test success after cleanup for clearer message * chore: cleanup makefile and always build op_geth * chore: remove unnecessary commit of transactor.sol * fix: add OP_GETH_LOCAL_DIRECTORY variable in .env file for developers to specify local op-geth directory * fix: run tests according to whether branch or local directory is specified * fix: run-test target local directory fix * chore: silence run-test target * fix: docker compose down only if .env file is present * chore: remove hardcoded values for env variables * fix: bugfix empty op_geth_image_tag * fix: reset op_geth_image_tag from example.env when building with submodule * fix * chore: make skipping op-stack and op-contracts env flags, clean up makefile * fix: revert unneccessary conductor retries * fix: remove tag changing logic * fix: change ports in docker compose

Include deposit tx type in `alloy_network::Network` impl

karlfloersch added the wip label Feb 25, 2020

willmeister approved these changes Feb 26, 2020

View reviewed changes

Remove nonces & fix bug with unset tx origin

3a33626

karlfloersch force-pushed the YAS-194/SkipSoliditySigVerify branch from 7ed5109 to 9f56d8a Compare March 16, 2020 17:20

karlfloersch removed the wip label Mar 16, 2020

karlfloersch added 2 commits March 18, 2020 16:41

Merge remote-tracking branch 'upstream/master' into YAS-194/SkipSolid…

6f73b6e

…itySigVerify

Fix illusive bug causing fullnode crash

b4060c8

karlfloersch force-pushed the YAS-194/SkipSoliditySigVerify branch from 9f56d8a to b4060c8 Compare March 19, 2020 00:33

Rename generateUnsignedCallCalldata->getTxCalldata

f11cf8f

K-Ho approved these changes Mar 19, 2020

View reviewed changes

Add comments

91f68cb

karlfloersch merged commit 7f07514 into ethereum-optimism:master Mar 19, 2020

InoMurko referenced this pull request in omgnetwork/optimism May 25, 2021

Merge pull request #23 from enyalabs/souradeep/modify_swaps

d1348a2

feat: update swaps

InoMurko referenced this pull request in omgnetwork/optimism May 25, 2021

Merge pull request #23 from enyalabs/souradeep/modify_swaps

fcf24bd

feat: update swaps

therealbytes added a commit to therealbytes/optimism that referenced this pull request May 17, 2024

Fix BigPreimageRegistry (ethereum-optimism#23)

ac33a98

* feat: add NewVersion util * fix: set big pi store radix and leaf * refactor: generalize pi registry test

bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024

feat: update tryFinalize() to check finality on each L2 block (ethe…

8d9427b

…reum-optimism#23)

bap2pecs pushed a commit to babylonlabs-io/optimism that referenced this pull request Jul 31, 2024

Add Jury mode and Jury sig submission (ethereum-optimism#23)

c7c5e08

SozinM pushed a commit to NethermindEth/optimism that referenced this pull request Feb 10, 2025

Merge pull request ethereum-optimism#23 from flashbots/pbs-readme

2f1c945

README for pbs enabled devnet

blockchaindevsh added a commit to blockchaindevsh/optimism that referenced this pull request Jun 30, 2025

enhance sgt e2e test (ethereum-optimism#23)

89839ba

theochap pushed a commit that referenced this pull request Dec 10, 2025

✨ HintWriter (#23)

f4c6315

Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025

fix: An error occurs if the requested amount is 0 ethereum-optimism#23

2aa922f

theochap pushed a commit that referenced this pull request Jan 15, 2026

Merge pull request #23 from alloy-rs/emhane/op-alloy-tx-type

8d054c2

Include deposit tx type in `alloy_network::Network` impl

theochap pushed a commit that referenced this pull request Jan 21, 2026

Merge pull request #23 from alloy-rs/emhane/op-alloy-tx-type

af0d21e

Include deposit tx type in `alloy_network::Network` impl

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove duplicate Solidity sig check in fullnode proxy#23

Remove duplicate Solidity sig check in fullnode proxy#23
karlfloersch merged 5 commits intoethereum-optimism:masterfrom
karlfloersch:YAS-194/SkipSoliditySigVerify

karlfloersch commented Feb 25, 2020 •

edited

Loading

Uh oh!

willmeister left a comment

Uh oh!

willmeister Feb 26, 2020

Uh oh!

K-Ho Mar 19, 2020

Uh oh!

K-Ho Mar 19, 2020

Uh oh!

K-Ho Mar 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -504,12 +502,13 @@ contract ExecutionManager is FullStateManager {
		* @param _timestamp The timestamp which should be used for this context.
		* @param _queueOrigin The queue which this context's transaction was sent from.

Conversation

karlfloersch commented Feb 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO

Metadata

Fixes

Contributing Agreement

Uh oh!

willmeister left a comment

Choose a reason for hiding this comment

Uh oh!

willmeister Feb 26, 2020

Choose a reason for hiding this comment

Uh oh!

K-Ho Mar 19, 2020

Choose a reason for hiding this comment

Uh oh!

K-Ho Mar 19, 2020

Choose a reason for hiding this comment

Uh oh!

K-Ho Mar 19, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

karlfloersch commented Feb 25, 2020 •

edited

Loading