-
Notifications
You must be signed in to change notification settings - Fork 158
upkeep #237
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
upkeep #237
Changes from all commits
fe675a1
754c8b6
2475d07
c97b76a
fe2fb39
a6aab82
4785c9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,13 @@ | ||
| // SPDX-License-Identifier: GPL-3.0-or-later | ||
|
|
||
| pragma solidity 0.8.4; | ||
|
|
||
| import {IERC20} from "openzeppelin-contracts-4.4.1/contracts/token/ERC20/IERC20.sol"; | ||
| import {IERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol"; | ||
| // solhint-disable-next-line max-line-length | ||
| import {SafeERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol"; | ||
| import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; | ||
|
|
||
| import {SafeERC20} from "openzeppelin-contracts-4.4.1/contracts/token/ERC20/utils/SafeERC20.sol"; | ||
| import {ERC20} from "openzeppelin-contracts-4.4.1/contracts/token/ERC20/ERC20.sol"; | ||
| // solhint-disable-next-line max-line-length | ||
| import {ERC20Permit} from "openzeppelin-contracts-4.4.1/contracts/token/ERC20/extensions/draft-ERC20Permit.sol"; | ||
| import {ERC20PermitUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol"; | ||
|
|
||
| /** | ||
| * @title WAMPL (Wrapped AMPL). | ||
|
|
@@ -26,8 +26,8 @@ import {ERC20Permit} from "openzeppelin-contracts-4.4.1/contracts/token/ERC20/ex | |
| * | ||
| * We call wAMPL the "wrapper" token and AMPL the "underlying" or "wrapped" token. | ||
| */ | ||
| contract WAMPL is ERC20, ERC20Permit { | ||
| using SafeERC20 for IERC20; | ||
| contract WAMPL is ERC20Upgradeable, ERC20PermitUpgradeable { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aalavandhan, what was the motivation to make this (and others, e.g. MedianOracle) upgradeable?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to use both the upgradable and non-upgradable versions of the openzeppelin contracts as dependencies, as effectively they are the same except how the storage layout is organized. Ultimately weather or not its upgradable (ie sits behind a proxy) depends on how its deployed. AMPL and the policy we use For wampl, median oracle and others we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the proxy-based upgradeability system, no constructors can be used, though. So you end up with this: constructor(address ampl) {
_ampl = ampl;
}
function init(string memory name_, string memory symbol_) public initializer {
__ERC20_init(name_, symbol_);
__ERC20Permit_init(name_);but what you really need is this (since it's not really upgradable) which is both idiomatic and beautiful constructor(
address ampl,
string memory name_,
string memory symbol_
) ERC20(name_, symbol_) ERC20Permit(name_) {
_ampl = ampl;Some of the bugs I've seen, originate from the fact that we (sometimes myself included) tend to program in a language vs into a language. It's just that using So why not just use both, at the end: "dependencies": {
+ "@openzeppelin/contracts": "^4.7.3",
"@openzeppelin/contracts-upgradeable": "^4.7.3"
},And if there are strong reasons for keeping things as-is, perhaps it'd be nice to see some slither checks. Absolutely love the project and your work, hope my comment didn't scare you! ❤️ 🍻 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Or, perhaps, since a potential V2 version of the contracts might be non-upgradable, why not just remove diff --git a/contracts/MedianOracle.sol b/contracts/MedianOracle.sol
index c201c8a..ce62860 100644
--- a/contracts/MedianOracle.sol
+++ b/contracts/MedianOracle.sol
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.4;
-import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import "./lib/Select.sol";
interface IOracle {
@@ -14,7 +14,7 @@ interface IOracle {
* @notice Provides a value onchain that's aggregated from a whitelisted set of
* providers.
*/
-contract MedianOracle is OwnableUpgradeable, IOracle {
+contract MedianOracle is Ownable, IOracle {
struct Report {
uint256 timestamp;
uint256 payload;
@@ -57,17 +57,16 @@ contract MedianOracle is OwnableUpgradeable, IOracle {
* @param minimumProviders_ The minimum number of providers with valid
* reports to consider the aggregate report valid.
*/
- function init(
+ constructor(
uint256 reportExpirationTimeSec_,
uint256 reportDelaySec_,
uint256 minimumProviders_
- ) public initializer {
+ ) public {
require(reportExpirationTimeSec_ <= MAX_REPORT_EXPIRATION_TIME);
require(minimumProviders_ > 0);
reportExpirationTimeSec = reportExpirationTimeSec_;
reportDelaySec = reportDelaySec_;
minimumProviders = minimumProviders_;
- __Ownable_init();
}
/**
diff --git a/contracts/WAMPL.sol b/contracts/WAMPL.sol
index dc412f1..2023856 100644
--- a/contracts/WAMPL.sol
+++ b/contracts/WAMPL.sol
@@ -1,13 +1,12 @@
// SPDX-License-Identifier: GPL-3.0-or-later
+
pragma solidity 0.8.4;
-import {IERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import {SafeERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
+import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
+import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import {ERC20PermitUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";
+import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
/**
* @title WAMPL (Wrapped AMPL).
@@ -26,8 +25,8 @@ import {ERC20PermitUpgradeable} from "@openzeppelin/contracts-upgradeable/token/
*
* We call wAMPL the "wrapper" token and AMPL the "underlying" or "wrapped" token.
*/
-contract WAMPL is ERC20Upgradeable, ERC20PermitUpgradeable {
- using SafeERC20Upgradeable for IERC20Upgradeable;
+contract WAMPL is ERC20, ERC20Permit {
+ using SafeERC20 for IERC20;
//--------------------------------------------------------------------------
// Constants
@@ -43,18 +42,15 @@ contract WAMPL is ERC20Upgradeable, ERC20PermitUpgradeable {
//--------------------------------------------------------------------------
/// @notice Contract constructor.
/// @param ampl The AMPL ERC20 token address.
- constructor(address ampl) {
- _ampl = ampl;
- }
-
- /// @notice Contract state initialization.
/// @param name_ The wAMPL ERC20 name.
/// @param symbol_ The wAMPL ERC20 symbol.
- function init(string memory name_, string memory symbol_) public initializer {
- __ERC20_init(name_, symbol_);
- __ERC20Permit_init(name_);
+ constructor(
+ address ampl,
+ string memory name_,
+ string memory symbol_
+ ) ERC20(name_, symbol_) ERC20Permit(name_) {
+ _ampl = ampl;
}
//--------------------------------------------------------------------------
@@ -237,7 +233,7 @@ contract WAMPL is ERC20Upgradeable, ERC20PermitUpgradeable {
uint256 amples,
uint256 wamples
) private {
- IERC20Upgradeable(_ampl).safeTransferFrom(from, address(this), amples);
+ IERC20(_ampl).safeTransferFrom(from, address(this), amples);
_mint(to, wamples);
}
@@ -255,13 +251,13 @@ contract WAMPL is ERC20Upgradeable, ERC20PermitUpgradeable {
) private {
_burn(from, wamples);
- IERC20Upgradeable(_ampl).safeTransfer(to, amples);
+ IERC20(_ampl).safeTransfer(to, amples);
}
/// @dev Queries the current total supply of AMPL.
/// @return The current AMPL supply.
function _queryAMPLSupply() private view returns (uint256) {
- return IERC20Upgradeable(_ampl).totalSupply();
+ return IERC20(_ampl).totalSupply();
}
//--------------------------------------------------------------------------
diff --git a/package.json b/package.json
index 32d611c..865af40 100644
--- a/package.json
+++ b/package.json
@@ -30,7 +30,7 @@
"lint"
],
"dependencies": {
- "@openzeppelin/contracts-upgradeable": "^4.7.3"
+ "@openzeppelin/contracts": "^4.7.3"
},
"devDependencies": {
"@ethersproject/abi": "^5.6.4",
diff --git a/test/unit/MedianOracle.ts b/test/unit/MedianOracle.ts
index ee2eb02..19636f6 100644
--- a/test/unit/MedianOracle.ts
+++ b/test/unit/MedianOracle.ts
@@ -22,8 +22,7 @@ async function setupContractsAndAccounts() {
C = accounts[3]
D = accounts[4]
factory = await ethers.getContractFactory('MedianOracle')
- oracle = await factory.deploy()
- await oracle.init(60, 10, 1)
+ oracle = await factory.deploy(60, 10, 1)
await oracle.deployed()
}
diff --git a/test/unit/WAMPL.ts b/test/unit/WAMPL.ts
index b78bf97..0d7cf8d 100644
--- a/test/unit/WAMPL.ts
+++ b/test/unit/WAMPL.ts
@@ -46,8 +46,8 @@ async function setupContracts() {
await ampl.setMonetaryPolicy(deployerAddress)
const wAMPLFactory = await ethers.getContractFactory('WAMPL')
- wAMPL = await wAMPLFactory.connect(deployer).deploy(ampl.address)
- await wAMPL.init(NAME, SYMBOL)
+ wAMPL = await wAMPLFactory.connect(deployer).deploy(ampl.address, NAME, SYMBOL)
}
describe('WAMPL', () => {
diff --git a/yarn.lock b/yarn.lock
index 2016367..0bdcdaa 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -798,10 +798,10 @@
"@types/sinon-chai" "^3.2.3"
"@types/web3" "1.0.19"
-"@openzeppelin/contracts-upgradeable@^4.7.3":
- version "4.7.3"
- resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.7.3.tgz#f1d606e2827d409053f3e908ba4eb8adb1dd6995"
- integrity sha512-+wuegAMaLcZnLCJIvrVUDzA9z/Wp93f0Dla/4jJvIhijRrPabjQbZe6fWiECLaJyfn5ci9fqf9vTw3xpQOad2A==
+"@openzeppelin/contracts@^4.7.3":
+ version "4.8.3"
+ resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.8.3.tgz#cbef3146bfc570849405f59cba18235da95a252a"
+ integrity sha512-bQHV8R9Me8IaJoJ2vPG4rXcL7seB7YVuskr4f+f5RyOStSZetwzkWtoqDMl5erkBJy0lDRUnIR2WIkPiC0GJlg==
"@openzeppelin/hardhat-upgrades@^1.19.0":
version "1.21.0" |
||
| using SafeERC20Upgradeable for IERC20Upgradeable; | ||
|
|
||
| //-------------------------------------------------------------------------- | ||
| // Constants | ||
|
|
@@ -43,15 +43,18 @@ contract WAMPL is ERC20, ERC20Permit { | |
|
|
||
| //-------------------------------------------------------------------------- | ||
|
|
||
| /// @notice Contract constructor. | ||
| /// @param ampl The AMPL ERC20 token address. | ||
| constructor(address ampl) { | ||
| _ampl = ampl; | ||
| } | ||
|
|
||
| /// @notice Contract state initialization. | ||
| /// @param name_ The wAMPL ERC20 name. | ||
| /// @param symbol_ The wAMPL ERC20 symbol. | ||
| constructor( | ||
| address ampl, | ||
| string memory name_, | ||
| string memory symbol_ | ||
| ) ERC20(name_, symbol_) ERC20Permit(name_) { | ||
| _ampl = ampl; | ||
| function init(string memory name_, string memory symbol_) public initializer { | ||
| __ERC20_init(name_, symbol_); | ||
| __ERC20Permit_init(name_); | ||
| } | ||
|
|
||
| //-------------------------------------------------------------------------- | ||
|
|
@@ -234,7 +237,7 @@ contract WAMPL is ERC20, ERC20Permit { | |
| uint256 amples, | ||
| uint256 wamples | ||
| ) private { | ||
| IERC20(_ampl).safeTransferFrom(from, address(this), amples); | ||
| IERC20Upgradeable(_ampl).safeTransferFrom(from, address(this), amples); | ||
|
|
||
| _mint(to, wamples); | ||
| } | ||
|
|
@@ -252,13 +255,13 @@ contract WAMPL is ERC20, ERC20Permit { | |
| ) private { | ||
| _burn(from, wamples); | ||
|
|
||
| IERC20(_ampl).safeTransfer(to, amples); | ||
| IERC20Upgradeable(_ampl).safeTransfer(to, amples); | ||
| } | ||
|
|
||
| /// @dev Queries the current total supply of AMPL. | ||
| /// @return The current AMPL supply. | ||
| function _queryAMPLSupply() private view returns (uint256) { | ||
| return IERC20(_ampl).totalSupply(); | ||
| return IERC20Upgradeable(_ampl).totalSupply(); | ||
| } | ||
|
|
||
| //-------------------------------------------------------------------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Initializable.sol"; | ||
| import "./IERC20.sol"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| /** | ||
| * @title ERC20 interface | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| /** | ||
| * @title Initializable | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Initializable.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| /** | ||
| * @title SafeMath | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "../Orchestrator.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.4.24; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "../MedianOracle.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Mock.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Mock.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Mock.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "./Mock.sol"; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pragma solidity 0.7.6; | ||
| pragma solidity 0.8.4; | ||
|
|
||
| import "../Orchestrator.sol"; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.