Skip to content

Optimism L2 - token [Hardhat - v2]#33

Merged
krebernisak merged 22 commits into
masterfrom
feature/oe-token
Aug 25, 2021
Merged

Optimism L2 - token [Hardhat - v2]#33
krebernisak merged 22 commits into
masterfrom
feature/oe-token

Conversation

@krebernisak
Copy link
Copy Markdown
Contributor

@krebernisak krebernisak commented Apr 27, 2021

  • Add OVM compilation target through Hardhat Optimism plugin
  • Support compilation of multiple Solidity versions (0.4, 0.6, 0.7...) and multiple targets (EVM, OVM,...)
  • Add @chainlink/optimism-utils a set of useful functions to interact with Optimism network
  • Add LinkTokenChild contract, an access-controlled mintable & burnable LinkToken, for use on sidechains and L2 networks
  • Include vendor contract sources as submodules (mitigate supply chain type of attacks)
  • Moved CI from Travis to GitHub Actions
  • Add CI integration tests running L1 & L2 networks
  • Add contract versioning
  • Add CHANGELOG information

@krebernisak krebernisak force-pushed the feature/oe-token branch 2 times, most recently from e4fb6db to e971321 Compare April 27, 2021 11:19
@krebernisak krebernisak force-pushed the feature/oe-token branch 2 times, most recently from e9afcef to 9464717 Compare April 27, 2021 18:01
@krebernisak krebernisak changed the title Optimism L2 integration [Hardhat - v2] Optimism L2 integration - token [Hardhat - v2] Apr 27, 2021
@krebernisak krebernisak changed the title Optimism L2 integration - token [Hardhat - v2] Optimism L2 - token [Hardhat - v2] Apr 27, 2021
- Update OZ lib to v3.4.0, polish contracts, fix tests
- Turn off solc bytecodeHash metadata generation
- Move continuous integration to GitHub
- Add target (OVM) support to getContractFactroy fn
- Add Optimism L2 network bootstrap to CI integration job
- Strictly declare Versions and Targets
- Integrating @chainlink/optimism-utils
- Update env loading and docs
- Use 1_000_000 optimizer runs for v0.7
- Add LinkTokenChild contract
- Run on self-hosted CI runner
- Add yargs lib to parse argv
- Add contract size and gas reporters
@krebernisak
Copy link
Copy Markdown
Contributor Author

@alex & @yos please check out the Hardhat/OVM integration here. It supports the compilation of multiple versions (v0.4, v0.6, ...) and multiple targets (EVM, OVM,...). We'd need this functionality in multiple Solidity projects, so an idea is to try to share a setup like this and include it everywhere. I'd appreciate your feedback.

@Steve & @kaleofduty the LinkTokenChild contract is a core part of the proposal that can enable us to move quickly to new networks, while still staying flexible and having an upgrade path when the full bridge is deployed. The proposal is documented here.

Comment thread package.json Outdated
"dependencies": {
"@chainlink/contracts": "^0.1.6",
"@openzeppelin/contracts": "^3.4.0"
"@chainlink/optimism-utils": "https://github.com/smartcontractkit/optimism-utils.git",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can't all of these other than OZ be moved into dev dependencies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"Needs to be" a proper dependency as this is now more than just a raw contracts package. It brings some useful functions for consumers like Gauntlet to use, for example, the getContractFactory fn to load the factory by name for a specific version & target.

Most of that code could be extracted in a separate lib, and then reused here, in Feeds Registry repo, in our core evm-contracts, etc.

The specific "@chainlink/optimism-utils" dependency is because of optimism.loadEnv fn which is currently repo specific and does not have much use externally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've cleaned this a little, but still left the @chainlink/optimism-utils as a dependency. It's less confusing to use it this way currently. I agree we can organize this better, and please share your suggestions, but I guess this is good enough for now.

I vote we try to see how all of this fits into our larger set of contract repos & tools. We can try to use it (share hardhat & optimism utils) in Gauntlet, Feed Registry, core contracts, and then restructure it to fit better depending on the outcome.

Copy link
Copy Markdown
Contributor Author

@krebernisak krebernisak May 7, 2021

Choose a reason for hiding this comment

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

Remove export of JS helper functions is the commit that addresses this.
JS code is not exported anymore and will be manually c/p to Gauntlet.

Comment thread contracts/v0.7/bridge/token/LinkTokenChild.sol
Copy link
Copy Markdown
Contributor

@se3000 se3000 left a comment

Choose a reason for hiding this comment

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

Probably not for this PR, but I think we should vendor our Solidity dependencies for this repo like we do for the others.

Comment thread contracts/v0.7/bridge/token/LinkTokenChild.sol Outdated
Comment thread contracts/v0.7/bridge/token/LinkTokenChild.sol
Comment thread contracts/v0.7/bridge/token/LinkTokenChild.sol
@krebernisak
Copy link
Copy Markdown
Contributor Author

krebernisak commented May 7, 2021

To see an example usage of the LinkTokenChild contract, in the context of Optimism, please check out the Optimism L2 - bridge #34 PR.

P.S. The CI fails because of upstream Optimism monorepo issues, locally works perfectly. Investigating...

Comment thread contracts/v0.6/mocks/Token677.sol Outdated
Comment thread contracts/vendor/@openzeppelin/contracts/3.4.1/commit.info Outdated
- By doing this we are trying to lower the probablity of supply chain type of attacks
- By doing this we are trying to lower the probablity of supply chain type of attacks
Comment thread test/v0.7/bridge/token/LinkTokenChild.test.ts Outdated
Comment thread test/v0.7/bridge/token/LinkTokenChild.test.ts Outdated
@se3000
Copy link
Copy Markdown
Contributor

se3000 commented May 11, 2021

Again, wondering if we should use submodules instead of manually copying a hash. Seems error prone and non-standard as is.

se3000
se3000 previously approved these changes May 12, 2021
@krebernisak krebernisak changed the base branch from master to chores/add-flattened-solidity June 9, 2021 10:35
@krebernisak krebernisak changed the base branch from chores/add-flattened-solidity to master June 9, 2021 10:35
Copy link
Copy Markdown

@lawrencexia lawrencexia left a comment

Choose a reason for hiding this comment

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

👍 to unblock merge - with the understanding that this branch has previously been reviewed, audited, and deployed

@krebernisak krebernisak merged commit 7ab441d into master Aug 25, 2021
@krebernisak krebernisak deleted the feature/oe-token branch August 25, 2021 11:14
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