-
Notifications
You must be signed in to change notification settings - Fork 157
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
Configurator code skeleton #12
Conversation
/** | ||
* @dev Emitted when the admin account has changed. | ||
*/ | ||
event AdminChanged(address previousAdmin, address newAdmin); |
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.
can we delete the AdminChanged
event?
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.
definitely, good catch
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.
removed 👍
@@ -4,19 +4,19 @@ | |||
|
|||
"@cspotcode/source-map-consumer@0.8.0": | |||
version "0.8.0" | |||
resolved "https://registry.yarnpkg.com/@cspotcode/source-map-consumer/-/source-map-consumer-0.8.0.tgz#33bf4b7b39c178821606f669bbc447a6a629786b" | |||
resolved "https://registry.npmjs.org/@cspotcode/source-map-consumer/-/source-map-consumer-0.8.0.tgz" |
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.
Just noticed these changes to yarn.lock
; did you intend to include this in the PR?
@@ -0,0 +1,22 @@ | |||
The MIT License (MIT) |
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.
🤔 Does this mean everything vendored needs to have the same license?
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.
Not sure it's in the scope of this task, but definitely great question for license discussion
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.
Nah, you're just following the rules of the MIT license, which is to retain it and nothing else. You can include this within any other license of code (including commercial only or unlicensed)
|
||
// We recommend this pattern to be able to use async/await everywhere | ||
// and properly handle errors. | ||
main() |
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.
I wonder can't we just use top-level await now with the version of node we are on?
|
||
import "./Config.sol"; | ||
|
||
contract ConfigFactory { |
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.
ProtocolFactory
@@ -0,0 +1,11 @@ | |||
pragma solidity ^0.8.0; | |||
|
|||
contract Config { |
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.
Configurator
|
||
event Values(uint targetReserves, uint borrowMin); | ||
|
||
constructor(address config_) { |
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.
uint alpha_, uint beta_, uint gramma_
} | ||
|
||
contract Protocol { | ||
ConfigInterface immutable config; |
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.
uint immutable alpha;
...
|
||
event NewConfig(address newConfig); | ||
|
||
function createConfig(uint targetReserves, uint borrowMin) external returns (address) { |
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.
createProtocol()
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.
new Protocol(config.targetReserves, config.borrowMin, ...)
pragma solidity ^0.8.0; | ||
|
||
contract Config { | ||
uint immutable public targetReserves; |
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.
Maybe these aren't immutable. and instead we have a "setTargetReserves()" function here.
|
||
event NewProtocol(address newProtocol); | ||
|
||
Configurator immutable configurator; |
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.
A bit curious if this should just be inlined?
Closing in lieu of #160 |
* Preliminary CometRewardsV2 functionality with MerkleProof * feat: restructorized contract storage, fixed all compilaion errors * fix: multiple assets compaign * fix: compaigns functionality * feat: Add finishAccrued and startAccrued bug fix (#5) * feat: claimed calculation fix * feat: claimed calculation fix * feat: add CometRewards V1 interaction * wip: tests and new contract version * wip: contract update with more tests * wip: new contract update and revert changes to CometRewards * wip: cover most of the contract and fix some bugs * wip: coverage improvement * feat: complited contract changes and test coverage * feat: remove package-lock * feat: set yarn file to the main branch version * feat: update yarn file
admin
to OZ standard transparent upgradeable proxy contracts, this commit b28e823config-script.ts