Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Migration scripts #191

Merged
merged 3 commits into from
Sep 22, 2020
Merged

Migration scripts #191

merged 3 commits into from
Sep 22, 2020

Conversation

haythemsellami
Copy link
Member

@haythemsellami haythemsellami commented Sep 20, 2020 •
edited
Loading

Migration scripts

High Level Description

This PR add migration script for Mainnet and Rinkeby network.

Code

  • Unit test 100% coverage
  • Does your code follow the naming and code documentation guidelines?

Documentation

  • Is your code up to date with the spec?
  • Have you added your tests to the testing doc?

@haythemsellami haythemsellami self-assigned this Sep 20, 2020
@haythemsellami haythemsellami changed the title [WIP] Migration scripts Migration scripts Sep 21, 2020
aparnakr
aparnakr reviewed Sep 22, 2020
View reviewed changes
migrations/2_deploy_contracts.js
Comment on lines +73 to +74
const oracle = await Oracle.deployed()
await addressbook.setOracle(oracle.address, {from: deployerAddress})
Copy link
Contributor

Choose a reason for hiding this comment

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

what about the usdc, compound and ETH pricers?

Copy link
Contributor

Choose a reason for hiding this comment

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

those need to be deployed, dispute + locking period set right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We should make some script to do any call like setting dispute period, adding pricer...etc as they are not mainly part of deployment.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, do you want to open an issue for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Already opened one #187

antoncoding
antoncoding approved these changes Sep 22, 2020
View reviewed changes
Copy link
Contributor

@antoncoding antoncoding 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, but why do we need to separate mainnet and rinkeby in both scripts? Aren't they the same?

aparnakr
aparnakr reviewed Sep 22, 2020
View reviewed changes
migrations/3_setup_ownership.js
await whitelist.transferOwnership(newOwner, {from: deployerAddress})
await oracle.transferOwnership(newOwner, {from: deployerAddress})
await pool.transferOwnership(newOwner, {from: deployerAddress})
await controller.transferOwnership(newOwner, {from: deployerAddress})
Copy link
Contributor

Choose a reason for hiding this comment

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

where does the setup of the emergnecy shutdown terminator and other access controls happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah same we should have a web3js script or a truffle script to setup that.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, want to open an issue for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

#187

@haythemsellami
Copy link
Member Author

Looks good, but why do we need to separate mainnet and rinkeby in both scripts? Aren't they the same?

Yeah for now they are the same, kept them separate for easier maintenance/update.

aparnakr
aparnakr reviewed Sep 22, 2020
View reviewed changes
Copy link
Contributor

@aparnakr aparnakr left a comment

Choose a reason for hiding this comment

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

Looks awesome! Hyped to get it up on testnet! left a couple of comments :D

aparnakr
aparnakr approved these changes Sep 22, 2020
View reviewed changes
Copy link
Contributor

@aparnakr aparnakr left a comment

Choose a reason for hiding this comment

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

Lgtm! :D

@haythemsellami haythemsellami merged commit 8246f9c into master Sep 22, 2020
@haythemsellami haythemsellami deleted the task/deployment-script branch September 22, 2020 14:02
aparnakr added a commit that referenced this pull request Sep 24, 2020
@aparnakr @haythem96 @antoncoding
Integration Test/naked calls (#195)
0d1cd07 
* add naked call tests

* add awaits

* Migration scripts (#191)

* init migration script

* migration script

* fix build

* add naked call tests

* add awaits

* move test to a before setup

* add naked call tests

* add awaits

* move test to a before setup

* add naked call tests

* add awaits

* Update test/integration-tests/nakedCallExpireOtm.test.ts

Co-authored-by: Anton Cheng <[email protected]>

* remove unspecified revert

Co-authored-by: Haythem Sellami <[email protected]>
Co-authored-by: Anton Cheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoncoding antoncoding antoncoding approved these changes

@aparnakr aparnakr aparnakr approved these changes

Assignees

@haythemsellami haythemsellami

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@haythemsellami @aparnakr @antoncoding