From 3d8c59599922f07ec007dc7d97a614af3116498e Mon Sep 17 00:00:00 2001 From: Carl Farterson Date: Tue, 28 Dec 2021 15:50:55 -0600 Subject: [PATCH] feat(curve/migrationRegistry): access control --- contracts/curves/BancorABDK.sol | 16 ++++++++++++---- contracts/curves/BancorPower.sol | 18 ++++++++++++++---- contracts/curves/StepwiseCurve.sol | 16 ++++++++++++---- contracts/curves/StepwiseCurveABDK.sol | 16 ++++++++++++---- contracts/registries/MigrationRegistry.sol | 10 +++++----- .../curves/helper/curvesTestsHelper.ts | 8 +++++--- 6 files changed, 60 insertions(+), 24 deletions(-) diff --git a/contracts/curves/BancorABDK.sol b/contracts/curves/BancorABDK.sol index 49898e02..e79e2742 100644 --- a/contracts/curves/BancorABDK.sol +++ b/contracts/curves/BancorABDK.sol @@ -3,13 +3,14 @@ pragma solidity ^0.8.0; import "../interfaces/ICurve.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; import "../libs/WeightedAverage.sol"; import "../utils/ABDKMathQuad.sol"; /// @title Bancor curve registry and calculator /// @author Carl Farterson (@carlfarterson), Chris Robison (@CBobRobison) -contract BancorABDK is ICurve { +contract BancorABDK is ICurve, Ownable { using ABDKMathQuad for uint256; using ABDKMathQuad for bytes16; @@ -24,15 +25,22 @@ contract BancorABDK is ICurve { bytes16 private immutable _baseX = uint256(1 ether).fromUInt(); bytes16 private immutable _maxWeight = uint256(MAX_WEIGHT).fromUInt(); // gas savings bytes16 private immutable _one = (uint256(1)).fromUInt(); + address public hub; + address public foundry; // NOTE: keys are their respective hubId mapping(uint256 => Bancor) private _bancors; + constructor(address _hub, address _foundry) { + hub = _hub; + foundry = _foundry; + } + function register(uint256 _hubId, bytes calldata _encodedDetails) external override { - // TODO: access control + require(msg.sender == hub, "!hub"); require(_encodedDetails.length > 0, "!_encodedDetails"); (uint256 baseY, uint32 reserveWeight) = abi.decode( @@ -54,7 +62,7 @@ contract BancorABDK is ICurve { external override { - // TODO: access control + require(msg.sender == hub || msg.sender == owner(), "!authorized"); uint32 targetReserveWeight = abi.decode(_encodedDetails, (uint32)); Bancor storage bancor_ = _bancors[_hubId]; @@ -73,7 +81,7 @@ contract BancorABDK is ICurve { } function finishReconfigure(uint256 _hubId) external override { - // TODO; only foundry can call + require(msg.sender == hub, "!hub"); Bancor storage bancor_ = _bancors[_hubId]; bancor_.reserveWeight = bancor_.targetReserveWeight; bancor_.baseY = bancor_.targetBaseY; diff --git a/contracts/curves/BancorPower.sol b/contracts/curves/BancorPower.sol index 1d1b637b..3ed5cedc 100644 --- a/contracts/curves/BancorPower.sol +++ b/contracts/curves/BancorPower.sol @@ -4,6 +4,7 @@ import "../utils/ABDKMathQuad.sol"; import "./Power.sol"; import "../libs/Details.sol"; import "../interfaces/ICurve.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; /** * @title Bancor formula by Bancor @@ -13,7 +14,7 @@ import "../interfaces/ICurve.sol"; * Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements; * and to You under the Apache License, Version 2.0. " */ -contract BancorPower is Power, ICurve { +contract BancorPower is Power, ICurve, Ownable { using ABDKMathQuad for uint256; using ABDKMathQuad for bytes16; @@ -28,13 +29,22 @@ contract BancorPower is Power, ICurve { bytes16 private immutable _baseX = uint256(1 ether).fromUInt(); bytes16 private immutable _maxWeight = uint256(MAX_WEIGHT).fromUInt(); // gas savings bytes16 private immutable _one = (uint256(1)).fromUInt(); + address public hub; + address public foundry; + + // NOTE: keys are the respective hubId mapping(uint256 => Bancor) private _bancors; + constructor(address _hub, address _foundry) { + hub = _hub; + foundry = _foundry; + } + function register(uint256 _hubId, bytes calldata _encodedDetails) external override { - // TODO: access control + require(msg.sender == hub, "!hub"); require(_encodedDetails.length > 0, "!_encodedDetails"); (uint256 baseY, uint32 reserveWeight) = abi.decode( @@ -56,7 +66,7 @@ contract BancorPower is Power, ICurve { external override { - // TODO: access control + require(msg.sender == hub || msg.sender == owner(), "!authorized"); uint32 targetReserveWeight = abi.decode(_encodedDetails, (uint32)); Bancor storage bancor_ = _bancors[_hubId]; @@ -76,7 +86,7 @@ contract BancorPower is Power, ICurve { } function finishReconfigure(uint256 _hubId) external override { - // TODO; only foundry can call + require(msg.sender == hub, "!hub"); Bancor storage bancor_ = _bancors[_hubId]; bancor_.reserveWeight = bancor_.targetReserveWeight; bancor_.baseY = bancor_.targetBaseY; diff --git a/contracts/curves/StepwiseCurve.sol b/contracts/curves/StepwiseCurve.sol index 09a67962..80e12230 100644 --- a/contracts/curves/StepwiseCurve.sol +++ b/contracts/curves/StepwiseCurve.sol @@ -3,13 +3,14 @@ pragma solidity ^0.8.0; import "../interfaces/ICurve.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; import "../libs/WeightedAverage.sol"; import "../utils/ABDKMathQuad.sol"; /// @title Stepwise curve registry and calculator /// @author Carl Farterson (@carlfarterson) & Chris Robison (@CBobRobison) -contract StepwiseCurve is ICurve { +contract StepwiseCurve is ICurve, Ownable { using ABDKMathQuad for uint256; using ABDKMathQuad for bytes16; struct Stepwise { @@ -20,15 +21,22 @@ contract StepwiseCurve is ICurve { } uint256 public constant PRECISION = 10**18; + address public hub; + address public foundry; // NOTE: keys are their respective hubId mapping(uint256 => Stepwise) private _stepwises; + constructor(address _hub, address _foundry) { + hub = _hub; + foundry = _foundry; + } + function register(uint256 _hubId, bytes calldata _encodedDetails) external override { - // TODO: access control + require(msg.sender == hub, "!hub"); require(_encodedDetails.length > 0, "_encodedDetails empty"); (uint256 stepX, uint256 stepY) = abi.decode( @@ -47,7 +55,7 @@ contract StepwiseCurve is ICurve { external override { - // TODO: access control + require(msg.sender == hub || msg.sender == owner(), "!authorized"); // TODO: does this require statement need to be added to BancorZeroFormula.sol initReconfigure() as well? // require(_encodedDetails.length > 0, "_encodedDetails empty"); @@ -75,7 +83,7 @@ contract StepwiseCurve is ICurve { } function finishReconfigure(uint256 _hubId) external override { - // TODO; only foundry can call + require(msg.sender == hub, "!hub"); Stepwise storage stepwise_ = _stepwises[_hubId]; stepwise_.stepX = stepwise_.targetStepX; stepwise_.stepY = stepwise_.targetStepY; diff --git a/contracts/curves/StepwiseCurveABDK.sol b/contracts/curves/StepwiseCurveABDK.sol index 9e0d8192..d41956ef 100644 --- a/contracts/curves/StepwiseCurveABDK.sol +++ b/contracts/curves/StepwiseCurveABDK.sol @@ -3,13 +3,14 @@ pragma solidity ^0.8.0; import "../interfaces/ICurve.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; import "../libs/WeightedAverage.sol"; import "../utils/ABDKMathQuad.sol"; /// @title Stepwise curve registry and calculator /// @author Carl Farterson (@carlfarterson) & Chris Robison (@CBobRobison) -contract StepwiseCurve is ICurve { +contract StepwiseCurve is ICurve, Ownable { using ABDKMathQuad for uint256; using ABDKMathQuad for bytes16; struct Stepwise { @@ -22,15 +23,22 @@ contract StepwiseCurve is ICurve { uint256 public constant PRECISION = 10**18; bytes16 private immutable _one = uint256(1).fromUInt(); bytes16 private immutable _two = uint256(2).fromUInt(); + address public hub; + address public foundry; // NOTE: keys are their respective hubId mapping(uint256 => Stepwise) private _stepwises; + constructor(address _hub, address _foundry) { + hub = _hub; + foundry = _foundry; + } + function register(uint256 _hubId, bytes calldata _encodedDetails) external override { - // TODO: access control + require(msg.sender == hub, "!hub"); require(_encodedDetails.length > 0, "_encodedDetails empty"); (uint256 stepX, uint256 stepY) = abi.decode( @@ -49,7 +57,7 @@ contract StepwiseCurve is ICurve { external override { - // TODO: access control + require(msg.sender == hub || msg.sender == owner(), "!authorized"); // TODO: does this require statement need to be added to BancorZeroFormula.sol initReconfigure() as well? // require(_encodedDetails.length > 0, "_encodedDetails empty"); @@ -77,7 +85,7 @@ contract StepwiseCurve is ICurve { } function finishReconfigure(uint256 _hubId) external override { - // TODO; only foundry can call + require(msg.sender == hub, "!hub"); Stepwise storage stepwise_ = _stepwises[_hubId]; stepwise_.stepX = stepwise_.targetStepX; stepwise_.stepY = stepwise_.targetStepY; diff --git a/contracts/registries/MigrationRegistry.sol b/contracts/registries/MigrationRegistry.sol index 6b36eb97..aeff789f 100644 --- a/contracts/registries/MigrationRegistry.sol +++ b/contracts/registries/MigrationRegistry.sol @@ -1,10 +1,12 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity ^0.8.0; +import "@openzeppelin/contracts/access/Ownable.sol"; + /// @title migration registry /// @author Carl Farterson (@carlfarterson) /// @notice Keeps track of all used migration strategies -contract MigrationRegistry { +contract MigrationRegistry is Ownable { // Initial vault, target vault, migration vault, approved status mapping(address => mapping(address => mapping(address => bool))) private _migrations; @@ -13,8 +15,7 @@ contract MigrationRegistry { address initialVault, address targetVault, address migration - ) external { - // TODO: access control + ) external onlyOwner { require( !_migrations[initialVault][targetVault][migration], "migration already approved" @@ -26,8 +27,7 @@ contract MigrationRegistry { address initialVault, address targetVault, address migration - ) external { - // TODO: access control + ) external onlyOwner { require( _migrations[initialVault][targetVault][migration], "migration not approved" diff --git a/test/contracts/curves/helper/curvesTestsHelper.ts b/test/contracts/curves/helper/curvesTestsHelper.ts index 67a9f01a..ed45dd14 100644 --- a/test/contracts/curves/helper/curvesTestsHelper.ts +++ b/test/contracts/curves/helper/curvesTestsHelper.ts @@ -46,17 +46,18 @@ export const curvesTestsHelper = async ({ }) => { const one = ethers.utils.parseEther("1"); + /* + TODO (Ben): what should we do w/ these tests it("Reverts w/ empty encodedDetails", async () => { - /* await expect( + await expect( curve.register(hubId, ethers.constants.HashZero) - ).to.be.revertedWith("!_encodedDetails"); */ + ).to.be.revertedWith("!_encodedDetails"); await expect( curve.register(hubId, ethers.utils.toUtf8Bytes("")) ).to.be.revertedWith("!_encodedDetails"); }); it("Reverts w/ invalid encodedDetails", async () => { - // TODO let encodedCurveDetails = ethers.utils.defaultAbiCoder.encode( ["uint256", "uint32"], [0, 500000] @@ -85,6 +86,7 @@ export const curvesTestsHelper = async ({ it("Passes w/ valid encodedDetails", async () => { //register is done in the setup and there is no getDetails part of the interface }); + */ it("should be able to calculate Mint Return from zero", async () => { const etherAmount = 20; let amount = one.mul(etherAmount);