feat: OVM #147
Merged
feat: OVM #147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
as EOAs are contracts in Optimism
Lower NonfungiblePositionManager bytecode size by 1250 bytes
…library Lower NonfungiblePositionManager bytecode size by 972 bytes
…library Lower NonfungiblePositionManager bytecode size by 523 bytes
Lower NonfungiblePositionManager bytecode size by 1483 bytes
…ary and make tokenURI function a library functionThis change made NonfungiblePositionManager contract size drop by just 83 bytes but more refactoring of this logic will follow to optimise that
…nManager contract size
…NonfungiblePositionManager contract size
…d. Save 177 bytes in NonfungiblePositionManager contract size
…tionManager bytecode decreased by 72 bytes
Saves 297 bytes from contract NonfungiblePositionManager contract size
Saves NonfungiblePositionManager 113 bytes off contract size
…t suite run This change was required to accommodate the new libraries that need to be linked to Uniswap V3 Core contracts. See the comments in test/shared/setup.ts for more info
…okenPositionDescriptor This was renamed to better describe what the test file does after library changes, but is renamed back here to make the diff more clear
|
merging this because of permissioning issues, it still needs some work |
NoahZinsmeister
pushed a commit
that referenced
this pull request
Jun 22, 2021
* Will drop eventually. Connect to the ovm compatible v3-core contracts * Ignore optimism folder from source control * Add hardhat-ovm package and config * Add npm scripts for compiling and testing against the OVM * Update to latest rebased with upstream changes core * Add scripts for starting/stopping the optimism services (copy from core) * Remove obsoleted Address.isContract check as EOAs are contracts in Optimism * Add @eth-optimism/hardhat-ovm and hardhat-contract-sizer to hardhat config * Update commit used for ovm-compatible uniswap v3 core contracts * Add to gitignore the ovm cache and artifacts folders * Add hardhat-contract-sizer package * Update PeripheryPayments.refundETH to use WETH instead of ETH * Add hardhat-dependency-compiler plugin and use it to help link libraries * Lower optimisation runs * Patch @OpenZeppelin Address library for compliance with OVM * Use WETH only for value transfer * Extract NonfungiblePositionManager.collect function into a library Lower NonfungiblePositionManager bytecode size by 1250 bytes * Extract NonfungiblePositionManager.decreaseLiquidity function into a library Lower NonfungiblePositionManager bytecode size by 972 bytes * Extract NonfungiblePositionManager.increaseLiquidity function into a library Lower NonfungiblePositionManager bytecode size by 523 bytes * Extract LiquidityManagement.addLiquidity function into a library Lower NonfungiblePositionManager bytecode size by 1483 bytes * Merge NonfungibleTokenPositionDescriptor with NonfungiblePositionLibrary and make tokenURI function a library functionThis change made NonfungiblePositionManager contract size drop by just 83 bytes but more refactoring of this logic will follow to optimise that * Do not recompose PoolKey struct. Saves 57 bytes in NonfungiblePositionManager contract size * Do not recompose PoolKey struct in mint function. Saves 102 bytes in NonfungiblePositionManager contract size * Remove mistakenly added empty file * Do not instantiate PoolKey struct in memory, reference storage instead. Save 177 bytes in NonfungiblePositionManager contract size * Optimise NonfungiblePositionLibrary.collect function. NonfungiblePositionManager bytecode decreased by 72 bytes * Remove parameterisation of WETH address and make that constant Saves 297 bytes from contract NonfungiblePositionManager contract size * Cleanup usage of the NonfungiblePositionLibrary * Optimise NonfungiblePositionLibrary.decreaseLiquidity function. Saves NonfungiblePositionManager 113 bytes off contract size * UniswapV3Pool and UniswapV3Factory are now only deployed once per test suite run This change was required to accommodate the new libraries that need to be linked to Uniswap V3 Core contracts. See the comments in test/shared/setup.ts for more info * Fix ABI imports * Fix WETH test which broke from changes in commit de5a8d0 * Fix deployment of MockTimeNonfungiblePositionManager * Add typechain-ovm folder to .gitignore * fix import * Update hardhat config with same adjustments made to Uniswap V3 Core * Fix deployment of NonfungiblePositionManager * Hardcode response to estimateGas to get more info on failures * Fix: when testing on OVM, use pre-deployed WETH instead of deploying it * Make NonfungiblePositionManager.poolIdToPoolKey mapping public * Refactor NonfungibleTokenPositionDescriptor tests into NonfungibleTokenPositionLibrary * Update @uniswap/v3-core to latest OVM-compatible commit * Reduce optimizer's number of runs * Update POOL_INIT_CODE_HASH - required due to update of v3-core contracts * Optimise increaseLiquidity function in NonfungiblePositionManager bytecode size decresed by 455 bytes * Move all events from INonfungiblePositionManager to NonfungiblePositionLibrary Decreased NonfungiblePositionManager bytecodesize by 301 bytes * Switch isAuthorizedForToken from modifier to a private function Saves 91 bytes in NonfungiblePositionManager bytecodesize * Extract logic for updating a position after minting Decreased NonfungiblePositionManager bytecode by 641 bytes * Update script to pull regenesis/0.4.0 branch * Restoring ETH opcodes: remove openzeppelin patch (undo 390ddbf) * Restoring ETH opcodes: Restore how PeripheryPayments handles ETH (undo 0fd3c46) * Restoring ETH opcodes: Undo test changes used to accomodate hardcoded WETH (undo b6b57c7 and f5b905d) * Restoring ETH opcodes: Add WETH back as an input parameter (undo 72c0000) * Switch l2geth and ovm solc version to WIP versions supporting ETH opcodes * Add WETH address constructor arg to MockTimeNonfungiblePositionManager * Fix token ordering in V3 migrator test (token input order must be sorted) * Restore isContract check so EVM tests pass (undo 42577c0) * Fix test that expected a revert but is no longer expected to revert * Remove estimateGas override - estimateGas now provides useful error messages * Add original WETH9 contract (sourced from https://etherscan.io/address/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2#code) * Modify WETH9 to support 0.7.6 and deploy it from compiled contract * Fix nonce too low error by awaiting each deploy instead of Promise.all() * Fix unsafe opcode from IncreaseLiquidity event * Fix various nonce too low errors by awaiting each call instead of Promise.all() * Add a 'Setup' test to ensure we always get the same bytecode hash * Update optimism repo and branch used * Remove unnecessary isContract check (undo 26519a2) On the OVM all accounts are contracts so this is not necessary. This change is also required to get contracts below the size limit * Update fixtures to only deploy WETH once to ensure safe address * Skip EOA tests since OVM has no EOAs * Fix nonce too low error by awaiting each call instead of Promise.all() * Fix for 'fully populated ticks' test reverting by using too much gas * Add instructions about how to run tests * Update to latest v3-core commit * Update branch used for optimism docker containers * Fix WETH9 to use a call instead of msg.sender.transfer() * Update run-optimism script to use the regenesis/0.4.0 branch * Rename NonfungibleTokenPositionLibrary test file back to NonfungibleTokenPositionDescriptor This was renamed to better describe what the test file does after library changes, but is renamed back here to make the diff more clear * Update expected revert string for permit test with invalid v value * Merge commit fix: Add WETH9 getter to TestNonfungiblePositionLibrary * Merge commit fix: Remove Promise.all and link library in Oracle test * Merge commit fix: Remove Promise.all, only run gas estimate assertions on EVM * Patch waffle's deployMockContract to support mock OVM contracts * Only run EOA permit tests on the OVM since they require EIP-1271 * Update multicall test to use two transactions to avoid exceeding OVM gas limit * Update README * Update OVM solc version (same build, cleaner syntax) * Automatically update POOL_INIT_CODE_HASH when running tests * Add CI job for OVM unit tests * Update @uniswap/v3-core dependency to point to Uniswap repo * Run prettier * deploy WETH in EVM, use 0x420..06 in OVM * Configure test reliant on hardcoded WETH address to only run on OVM * Run prettier * Skip V3 migrator tests on OVM * Don't fail if there is no backup file created by sed (for CI) * Update EVM snapshots for CI (OVM CI tests update snapshots) Co-authored-by: elenadimitrova <elena@arenabg.com> Co-authored-by: Kevin Ho <kevinjho1996@gmail.com>
Closed
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The OVM is a sandboxed environment for the EVM which allows execution of smart contract code on L2, with the ability to dispute it on L1 via a fraud proof. Solidity code can be compiled directly to OVM code instead of EVM code by using "optimistic solc", a fork of the well-known solc. The OVM bytecode is ~30% larger than EVM bytecode, which may result in the bytecode being larger than the 24kb size limit for smart contracts.
This PR refactors the codebase to allow it to fit in the 24kb limit, and does a minimal diff to the test suite so that tests can pass when ran against an instance of go-ethereum running the OVM. We suspect the test suite changes around waiting for txs to be confirmed would've been required anyway if we wanted to test against geth or any full node impl.
Explanation of code changes
permitsupport was removed since all OVM wallets are EIP-1271 compatible contract walletsgetPopulatedTicksInWorduses too much gas sogetPopulatedTicksInPartialWordwas addedNonfungiblePositionManagerbelow 24 kb include:NonfungiblePositionLibraryand out ofNonfungiblePositionManager.NonfungiblePositionLibrarybut still parameterized elsewhereNonfungiblePositionManager.positions()function and use the getter generated on a publicpositionsmappingNonfungibleTokenPositionDescriptorintoNonfungiblePositionLibraryand maketokenURIfunction a library function (commit 1f70e13)PoolKeystruct in memory and uses storage references instead (commit 8a4f929)isAuthorizedForTokenmodifier to a private function (commit 1e284f3)hardhat-dependency-compilerplugin to compile the V3 Core contracts for the OVM since those artifacts are required for testingPromise.all()usage when sending transactions to ensure nonces are properly updatedscripts/run-optimism.shscript which will clone the Optimism monorepo, build it, and bring up the network_Setup.spec.tstest file which must always be run first. This ensures library addresses (which are dependent on account nonce) are always the same, which ensures the bytecode hash is the same on each test runset-hash:evmandset-hash:ovmscripts to update the hardcoded bytecode hash inPoolAddress.sol. These are run automatically before running testsv3CoreFactoryFixtureso it's only run once per test suitedeployMockContractso it supports mock OVM contractsREADME.mdon how to run the EVM and OVM tests to accomodate thisStatus
All tests pass both in the EVM and in the OVM, with the exception of
V3Migrator, which is not required to be deployed on the OVM. A few tests are skipped:owned by eoapermit tests are skipped on the EVM since only EIP-1271 contract wallets are supporteddisplays ETH as token symbol for WETH tokenare skipped on the EVM since it fails due to the hardcoded WETH9 address inNonfungiblePositionLibraryV3Migratortests are skipped on the OVM. They will fail on the OVM since it tries to deploy EVM bytecode. (If V3Migrator is needed, the current blocker is adding support to the@eth-optimism/hardhat-ovmplugin to support compiling with multiple OVM solc versions)Due to the lack of snapshots and running against a real node, tests take much longer, ~40mins.
h/t @elenadimitrova @K-Ho @ben-chain @smartcontracts @gakonst for their help