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
Adds a command that deploys an OVM pair of instances on local chains #954
Conversation
publish/src/Deployer.js
Outdated
abi: compiled.abi, | ||
params: args, | ||
}); | ||
if (!areBytesSafeForOVM(encodedParameters)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be thrown before the contract is even created no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, I split the deployment call and put this in between because I initially intended to retrieve the encoding from the bytecode itself.
Will change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
publish/src/Deployer.js
Outdated
if (this.useOvm && !areBytesSafeForOVM(deployedContract.options.address)) { | ||
console.log( | ||
yellow( | ||
`⚠ WARNING: Deployed contract address ${deployedContract.options.address} contains unsafe opcodes for OVM. If it is used as a constructor argument in the deployment of another contract, it will make that deployment fail.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a retry function here? Or perhaps just throw and force a re-run of the deployment script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We thought about this... the thing is, we might retry for a contract whose address is not used as a deployment parameter.
What could work is try to predict the address of the contract-to-be using the EOA and its nonce. If the test fails, have the EOA make a "blank" transaction to increase its nonce and try again.
However, this might prove to be overkill since its not only addresses in constructor parameters that can trigger the second order effect failure, for example the number 91
, which is 0x5b
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented!
The deployer will now use dummy txs to increment nonces and produce safe contract addresses when deploying to OVM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to deploy-ovm-test-pair
* Sending rewards into L2 * Fixing broken parts * Unit tests * New unit tests * Adding coverage for migrate * Remove duplicate tests * Use smock to test L2 contracts (#820) * Remove MintableSynthetixMock.sol * Remove test:multi (implemented in external PR) * Remove secondary contract refs (#823) * New messenger interface (#825) * Add initial tests * Add loop for initializing the array * Add test for importVestingEntries() * Comment out expected data tests * Import new IRewardEscrowV2 interface * Fix SynthetixBridgeToOptimism unit tests * Fix SynthetixBridgeToBase unit tests * Add more tests * Use V2 suffix for rewardEscrow view function * Add RewardEscrowV2 for mocking * Minor fix * Adds a new script to connect l2 bridge (#894) * Adds a new script to connect l2 bridge * Bugfix on new command * Skip vsynth prod test (#895) * Adds a --clean-build flag to the build command (#896) * Fix lint pre-commit hook (#900) * Disable etherscan checks in CI (#897) * Skip vsynth prod test (#902) * Adds a few options to connect-bridge (#906) * Adds a few options to connect-bridge * Fix connect-bridge pks * Minor fix * Another minor fix * Fix * Logging more output * Fix * Fix * Fixing prod test for vSynth and verifying more rinkeby contracts in etherscan * remove aggregator feed for BCH prices (#905) * Prepublish step * 2.32.2-sip-95-remove-BCH-aggregator * Prepublish step * 2.32.3 * Develop - Merge master into develop (#911) * Address list renamed to address set and make adding elements idempotent. (#917) * Preparations for L2 Phase C deployment (#901) * Improved lint pre-commit hook (#918) * Build circleci config with Mustache (#883) * L2 Phase C Release (#919) * Prepublish step * 2.33.0-ovm * Updating versions for ovm * Add circleci paths to prettierignore for pretty-quick (#921) * Do not ignore dotfiles in js linter (#923) * reset iETH prices (#922) * reset iETH prices * update StakingRewardsiETH address and ABI * merge master into develop (#926) * reset iETH prices (#922) and update StakingRewardsiETH address and ABI Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: justin j. moses <justinjmoses@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> * Prepublish step * 2.33.1 * Added dockerhub authetication to circleci (#924) * Improved connect-bridge script (#928) * Use new rewardEscrowV2 * Modify burnForMigration() to return a strcut array * Use structs to avoid stack too deep errors * Use new interface * Use new importedVestingEntries() and fix unit tests * sip99 (#936) * Prepublish step * 2.33.2 * sip 98 double swing trade exchange fees (#935) * update calculation of exchangeFeeRate for swing trades from long <-> short synths without sUSD step * fix audit findings on missing exchangeFeeRate assignment and update Exchanger tests * release adhara 2.33.0 adhara (#939) * release adhara kovan 2.33.0 * release Exchanger 2.33.0 Adhara to mainnet * Prepublish step * 2.33.0 * Prepublish step * 2.34.0 * Version fix for master along with some fork fixes (#941) * Fixing versions * merge 2.34.0 release into develop (#940) * Add 0-debt requirement for withdrawals (#942) * Deploy bug fix: adding synths (#920) * Deployed fix for SynthetixBridgeToBase (#945) * Prepublish step * 2.34.1-ovm * reset iSynths (#946) * Prepublish step * 2.34.2 * Adding Shaula entry on releases * Prepublish step * 2.35.0-ovm * Disable coverage in ci until its fixed (#950) * SIP 100 Resolver & cache improvements (#951) * Adding a new command to deploy an ovm instance pair for testing * Minor lint fix * Progress detecting unsafe deployments * More robust OVM safety checks * First OVM local deploy * Working * Return the right messenger addresses * Predict deploy addresses in L2 deploys * Tidier output in connect-bridge * Remove log * Add new logic and fix tests * Use alt key * Refactoring L2 prod tests * Update interfaces * Deposits working in L2 tests * Progress with L2 withdrawal tests * Linter fixes * Withdrawals working in L2 prod tests * Refactor depositAndMigrateEscrow() and add unit tests * Add extra safety checks * Use new addressResolver * Deploy RewardEscrowV2 on local and local-ovm * Connect-bridge command: Check if resolver needs imports and if bridge needs sync separately (#944) * Prevent withdrawals on a suspended L2 system (#949) * Prevent withdrawals on a suspended L2 system * Also mocking SystemStatus on MintableSynthetix tests Co-authored-by: justin j. moses <justin@synthetix.io> * Adds a command that deploys an OVM pair of instances on local chains (#954) * Adding a new command to deploy an ovm instance pair for testing * Minor lint fix * Progress detecting unsafe deployments * More robust OVM safety checks * First OVM local deploy * Working * Predict deploy addresses in L2 deploys * Tidier output in connect-bridge * Remove log * Renamed some L2 functions (#961) * New command: migrate-bridge (#925) * Linter fixes * Migrate bridge cmd uses getVersions * Get latest bridge version directly from deployments * Update to develop * Added SystemSettings.crossDomainMessageGasLimit (#962) * Added a system setting * Minor fixes * Added FlexibleStorage in L2 bridge unit tests * Updates deploy-ovm-pair cmd to latest optimism-integration (#964) * WIP: add initial deployment and tests * Update interface * Build ci * Comment out tests * Comment out test file Co-authored-by: justin j. moses <justinjmoses@gmail.com> Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: Jackson <jackosmacko@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> Co-authored-by: Anton Jurisevic <zyzek@users.noreply.github.com> Co-authored-by: Clinton Ennis <clinton@synthetix.io> Co-authored-by: justin j. moses <justin@synthetix.io>
* Adds a new script to connect l2 bridge (#894) * Adds a new script to connect l2 bridge * Bugfix on new command * Skip vsynth prod test (#895) * Adds a --clean-build flag to the build command (#896) * Fix lint pre-commit hook (#900) * Disable etherscan checks in CI (#897) * Skip vsynth prod test (#902) * Adds a few options to connect-bridge (#906) * Adds a few options to connect-bridge * Fix connect-bridge pks * Minor fix * Another minor fix * Fix * Logging more output * Fix * Fix * Fixing prod test for vSynth and verifying more rinkeby contracts in etherscan * remove aggregator feed for BCH prices (#905) * Prepublish step * 2.32.2-sip-95-remove-BCH-aggregator * Prepublish step * 2.32.3 * Develop - Merge master into develop (#911) * Address list renamed to address set and make adding elements idempotent. (#917) * Preparations for L2 Phase C deployment (#901) * Improved lint pre-commit hook (#918) * Build circleci config with Mustache (#883) * L2 Phase C Release (#919) * Prepublish step * 2.33.0-ovm * Updating versions for ovm * Add circleci paths to prettierignore for pretty-quick (#921) * Do not ignore dotfiles in js linter (#923) * reset iETH prices (#922) * reset iETH prices * update StakingRewardsiETH address and ABI * merge master into develop (#926) * reset iETH prices (#922) and update StakingRewardsiETH address and ABI Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: justin j. moses <justinjmoses@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> * Prepublish step * 2.33.1 * Added dockerhub authetication to circleci (#924) * Improved connect-bridge script (#928) * sip99 (#936) * Prepublish step * 2.33.2 * sip 98 double swing trade exchange fees (#935) * update calculation of exchangeFeeRate for swing trades from long <-> short synths without sUSD step * fix audit findings on missing exchangeFeeRate assignment and update Exchanger tests * release adhara 2.33.0 adhara (#939) * release adhara kovan 2.33.0 * release Exchanger 2.33.0 Adhara to mainnet * Prepublish step * 2.33.0 * Prepublish step * 2.34.0 * Version fix for master along with some fork fixes (#941) * Fixing versions * merge 2.34.0 release into develop (#940) * Add 0-debt requirement for withdrawals (#942) * Deploy bug fix: adding synths (#920) * Deployed fix for SynthetixBridgeToBase (#945) * Prepublish step * 2.34.1-ovm * reset iSynths (#946) * Prepublish step * 2.34.2 * Adding Shaula entry on releases * Prepublish step * 2.35.0-ovm * Disable coverage in ci until its fixed (#950) * SIP 100 Resolver & cache improvements (#951) * Connect-bridge command: Check if resolver needs imports and if bridge needs sync separately (#944) * Prevent withdrawals on a suspended L2 system (#949) * Prevent withdrawals on a suspended L2 system * Also mocking SystemStatus on MintableSynthetix tests Co-authored-by: justin j. moses <justin@synthetix.io> * Adds a command that deploys an OVM pair of instances on local chains (#954) * Adding a new command to deploy an ovm instance pair for testing * Minor lint fix * Progress detecting unsafe deployments * More robust OVM safety checks * First OVM local deploy * Working * Predict deploy addresses in L2 deploys * Tidier output in connect-bridge * Remove log * Renamed some L2 functions (#961) * New command: migrate-bridge (#925) * Linter fixes * Migrate bridge cmd uses getVersions * Get latest bridge version directly from deployments * Update to develop * Added SystemSettings.crossDomainMessageGasLimit (#962) * Added a system setting * Minor fixes * Added FlexibleStorage in L2 bridge unit tests * Updates deploy-ovm-pair cmd to latest optimism-integration (#964) Co-authored-by: Alejandro Santander <Palebluedot@gmail.com> Co-authored-by: justin j. moses <justinjmoses@gmail.com> Co-authored-by: Jackson <jackosmacko@gmail.com> Co-authored-by: Synthetix Team <team@synthetix.io> Co-authored-by: Anton Jurisevic <zyzek@users.noreply.github.com> Co-authored-by: Clinton Ennis <clinton@synthetix.io> Co-authored-by: justin j. moses <justin@synthetix.io>
Assuming that docker is used to start a local L1 chain and a local L2 chain using https://github.com/ethereum-optimism/optimism-integration, this PR adds a
deploy-ovm-pair
command that deploys and connects a pair of instances to be used for local production tests.The PR also introduces OVM safety checking in the deployment process, to ensure that no dangerous
5b
orJUMPDEST
opcodes exist in appended deployment constructor parameters.Another interesting addition of the PR, is the ability for the deployment script to split addresses being imported in the AddressResolver into chunks. This was needed for OVM to avoid that call to exceed the block limit.
To try the script out:
git clone git@github.com:ethereum-optimism/optimism-integration.git
cd optimism-integration
docker-compose pull
./up.sh
cd ~/synthetix
(or wherever)node publish deploy-ovm-pair