Skip to content

Adds (unoptimized) functions to MerkleTrie.sol#160

Merged
karlfloersch merged 12 commits intomasterfrom
YAS-417/MerkleTrie/update
Jun 24, 2020
Merged

Adds (unoptimized) functions to MerkleTrie.sol#160
karlfloersch merged 12 commits intomasterfrom
YAS-417/MerkleTrie/update

Conversation

@smartcontracts
Copy link
Contributor

@smartcontracts smartcontracts commented Jun 18, 2020

Description

Helloooo. This PR adds two new (unoptimized) methods to MerkleTrie.sol, verifyExclusionProof and update. I managed to refactor verifyInclusionProof so most of the logic is shared between the three methods.

Unfortunately, the trie protocol is somewhat complex and, as a result, the code is too. I've also had to execute everything in an iterative manner because recursion tends to eat gas. Apologies in advance for the heavily nested conditionals :-/. I've added comments at the top level of each function and at various points within the most complicated functions.

Questions

  • Does anyone see any obvious optimizations we can make without significantly reducing code quality?

Metadata

Fixes

Contributing Agreement

@smartcontracts smartcontracts changed the title Adds (unoptimized) functions to MerkleTrie.sol [WIP] Adds (unoptimized) functions to MerkleTrie.sol Jun 18, 2020
Copy link

@willmeister willmeister left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

Comment on lines +298 to +299
TrieNode[] memory newNodes = new TrieNode[](3);
uint256 totalNewNodes = 0;

Choose a reason for hiding this comment

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

I think you can split the declaration of newNodes and its initialization, initializing it to the correct size in the if, else if and else, respectively. This would eliminate the need for totalNewNodes and its incrementing (or perhaps only doing it in the else).

One benefit to compartmentalizing this is that you can split the if / else if / else into helper functions that self-document.

Copy link
Contributor

@karlfloersch karlfloersch left a comment

Choose a reason for hiding this comment

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

@kfichter 🔥🔥🔥 LGTM! Feel free to hit (squash and) merrrrrge!

@karlfloersch
Copy link
Contributor

Merging!

@karlfloersch karlfloersch merged commit f810f3b into master Jun 24, 2020
@gakonst gakonst deleted the YAS-417/MerkleTrie/update branch March 18, 2021 15:08
snario pushed a commit that referenced this pull request Apr 14, 2021
* Separates storage from SCC and CTC (#151)

* First pass version

* More minor tweaks for tests to pass

* Add authentication

* Minor config updates

* Fix lint error

* Fix naming changes per review

* Enable Deployer Whitelist (#119)

* first pass, test runner updated

* add ability to only validate flag, test passes

* all tests passing

* clean up console.logs

* enforce gas refund preservation

* more cleanup/import removal

* whitelisted -> allowed

* first pass, test runner updated

* add ability to only validate flag, test passes

* all tests passing

* clean up console.logs

* enforce gas refund preservation

* more cleanup/import removal

* whitelisted -> allowed

* remove whitespace

* Restrict StateTransitionerFactory (#140)

* added msg sender check

* add create test

* cleanup

* add param

* add addressmanager.address param

* CTC Chain Monotonicity Fixes (#93)

* [wip] Fix block time logic

* some sad path and happy tests passing

* more progress

* first pass sad cases tested

* cleanup, adding empty tests

* more reversion tests

* rename shouldstartat}

* add final couple tests

* enable more tests

* cleanup

* remove .only

* textual cleanup

* make queue length public

* improve structure, comments

* update deploy config

* address nits

Co-authored-by: Karl Floersch <[email protected]>

* fix declarations, lint (#152)

* Adds river's new Merkle tree implementation, with some cleanup (#148)

* Reverts an accidental breaking merge

* Added new merkle tree impl

* add comments

* Final cleanups and merge

Co-authored-by: Ben Jones <[email protected]>

* Fix run gas Lower Bound (#94)

* added the check

* add test

* lower OVM TX size for Kovan

* re-remove gas check

* update gas vals slightly

* lint

* lint

* Merge master into freeze integration branch  (#153)

* update solidity version to ^0.7.0 (#122)

* update solc version to ^0.7.0

* interfaces back to solidity >0.6.0 <0.8.0

* update solc to 0.7.6

* back to 0.7.4

* upgrade to 0.7.6, fix EXTCODESIZE check

* versions >0.5.0 <0.8.0 for xdomain msgers

* ctc: disable appendQueueBatch (#150)

* ctc: disable appendSequencerBatch

* typo: fix

* re-enable verifyQueueTransaction test:

* add explicit test for verifying queue elements against either append

Co-authored-by: Ben Jones <[email protected]>

* fix up test

* remove .only

Co-authored-by: Alina <[email protected]>
Co-authored-by: Mark Tyneway <[email protected]>

* add check, simple test, update deploy (#154)

* go back to first name (#155)

* lint

* fix js number error

* add error logging to help debug deploy

* [code freeze] Fix deploy script (#156)

* fix deploy script

* add block time config

* ensure value is integer

* lint

* remove console logs from deploy

* Moves gas check to applyTransaction (#161)

* move to OVM_ST, pass test

* remove old test because functionality moved

* linting

* remove leaf hasing

* use safe EXEMRG wrapper (#162)

* use safeREQUIRE

* add owner getter

* relayer: add to config (#160)

* relayer: add to config

* lint: fix

* Fix minor error in test config

Co-authored-by: Kelvin Fichter <[email protected]>
Co-authored-by: ben-chain <[email protected]>
Co-authored-by: Alina <[email protected]>
Co-authored-by: Mark Tyneway <[email protected]>
Co-authored-by: Kevin Ho <[email protected]>
ClaytonNorthey92 added a commit to hemilabs/optimism that referenced this pull request Jul 1, 2024
526787acd popm/wasm: improve js.Value creation, add JSMarshaler interface (ethereum-optimism#160)
be5de3b9a popm/wasm: add error codes (ethereum-optimism#154)

git-subtree-dir: heminetwork
git-subtree-split: 526787acd0454b26bd4f44927aadcdda4bbdac58
blockchaindevsh added a commit to blockchaindevsh/optimism that referenced this pull request Jan 25, 2025
…-optimism#160)

* revert pip install

* fix resource_class

* fix build dac server

* fix contracts-bedrock-coverage

* remove contracts-bedrock-coverage that add by mistake merge

* use submodule instead of pull

* refactor

* revert refactor

* fix  semver-lock.json

* lint

* fix snapshots

* update gas snapshot

* fix go run ./scripts/checks/interfaces

* fix branch

* fix TestDencunBlobTxInclusion

* fix DencunBlobTx

* try remove docker

* revert to use docker

* fix unexpected status code: 415

* use public mainnet rpc

* simplified workflow

* use machine: true

* Revert "use machine: true"

This reverts commit 91339ef.

* merge

* leave go-tests only

* wait longer

* hardcode DAC

* fix dac.urls

* wait longer for op-geth verifier

* debug

* debug

* debug

* disable l2blob

* revert

* debug

* debug

* debug

* debug

* revert

* fix url check

* revert log

* revert log

* revert ctx

* open contract tests

* fix circleci-agent

* uncomment tests

* test

* remove split

* fix circleci-agents

* temporarily skip test

* fix timeout

* open kurtosis

* change exit code

---------

Co-authored-by: blockchaindevsh <[email protected]>
ClaytonNorthey92 added a commit to hemilabs/optimism that referenced this pull request Apr 4, 2025
526787acd popm/wasm: improve js.Value creation, add JSMarshaler interface (ethereum-optimism#160)
be5de3b9a popm/wasm: add error codes (ethereum-optimism#154)

git-subtree-dir: heminetwork
git-subtree-split: 526787acd0454b26bd4f44927aadcdda4bbdac58
ClaytonNorthey92 added a commit to hemilabs/optimism that referenced this pull request Apr 7, 2025
526787acd popm/wasm: improve js.Value creation, add JSMarshaler interface (ethereum-optimism#160)
be5de3b9a popm/wasm: add error codes (ethereum-optimism#154)

git-subtree-dir: heminetwork
git-subtree-split: 526787acd0454b26bd4f44927aadcdda4bbdac58
falcorocks added a commit that referenced this pull request Dec 2, 2025
Testing asterisc image built with factory workflow from PR #160
to verify ARM64 compatibility and resolve build warnings.
Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025
theochap pushed a commit that referenced this pull request Jan 15, 2026
### Description

Elides lifetimes to fix nightly lints.
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.

3 participants