From 9c0a213e957fd2d64b5faff5b9e7102cccd49ee6 Mon Sep 17 00:00:00 2001 From: Karl Kempe Date: Tue, 13 Sep 2022 21:44:12 +0000 Subject: [PATCH] ethereum: remove proxy --- ethereum/contracts/gasOracle/GasOracle.sol | 30 +++- .../contracts/gasOracle/GasOracleGetters.sol | 10 +- .../gasOracle/GasOracleGovernance.sol | 35 ----- .../gasOracle/GasOracleImplementation.sol | 29 ---- .../contracts/gasOracle/GasOracleProxy.sol | 15 -- .../contracts/gasOracle/GasOracleSetters.sol | 8 +- .../contracts/gasOracle/GasOracleSetup.sol | 29 ---- .../contracts/gasOracle/GasOracleState.sol | 12 +- ethereum/forge-test/GasOracle.t.sol | 139 +++++++----------- 9 files changed, 97 insertions(+), 210 deletions(-) delete mode 100644 ethereum/contracts/gasOracle/GasOracleGovernance.sol delete mode 100644 ethereum/contracts/gasOracle/GasOracleImplementation.sol delete mode 100644 ethereum/contracts/gasOracle/GasOracleProxy.sol delete mode 100644 ethereum/contracts/gasOracle/GasOracleSetup.sol diff --git a/ethereum/contracts/gasOracle/GasOracle.sol b/ethereum/contracts/gasOracle/GasOracle.sol index baac11a..35cb242 100644 --- a/ethereum/contracts/gasOracle/GasOracle.sol +++ b/ethereum/contracts/gasOracle/GasOracle.sol @@ -5,13 +5,26 @@ pragma solidity ^0.8.0; import "./GasOracleGetters.sol"; import "./GasOracleSetters.sol"; -import "./GasOracleGovernance.sol"; -abstract contract GasOracle is GasOracleGovernance { +contract GasOracle is GasOracleGetters, GasOracleSetters { struct UpdatePrice { uint16 chainId; - uint256 gasPrice; - uint256 nativeCurrencyPrice; + uint128 gasPrice; + uint128 nativeCurrencyPrice; + } + + constructor(address wormhole, uint16 srcChainId) { + setupInitialState(_msgSender(), wormhole, srcChainId); + } + + function setupInitialState(address owner, address wormhole, uint16 srcChainId) internal { + require(owner != address(0), "owner == address(0)"); + setOwner(owner); + require(srcChainId > 0, "srcChainId == 0"); + setChainId(srcChainId); + // might use this later to consume price data via VAAs? + require(wormhole != address(0), "wormhole == address(0)"); + setWormhole(wormhole); } function getPrice(uint16 targetChainId) public view returns (uint256 quote) { @@ -21,10 +34,10 @@ abstract contract GasOracle is GasOracleGovernance { uint256 dstNativeCurrencyPrice = nativeCurrencyPrice(targetChainId); require(dstNativeCurrencyPrice > 0, "dstNativeCurrencyPrice == 0"); - quote = gasPrice(targetChainId) * dstNativeCurrencyPrice / srcNativeCurrencyPrice; + quote = (gasPrice(targetChainId) * dstNativeCurrencyPrice) / srcNativeCurrencyPrice; } - function updatePrice(uint16 updateChainId, uint256 updateGasPrice, uint256 updateNativeCurrencyPrice) + function updatePrice(uint16 updateChainId, uint128 updateGasPrice, uint128 updateNativeCurrencyPrice) public onlyOwner { @@ -43,4 +56,9 @@ abstract contract GasOracle is GasOracleGovernance { } } } + + modifier onlyOwner() { + require(owner() == _msgSender(), "owner() != _msgSender()"); + _; + } } diff --git a/ethereum/contracts/gasOracle/GasOracleGetters.sol b/ethereum/contracts/gasOracle/GasOracleGetters.sol index d20ea08..c7a8758 100644 --- a/ethereum/contracts/gasOracle/GasOracleGetters.sol +++ b/ethereum/contracts/gasOracle/GasOracleGetters.sol @@ -7,7 +7,7 @@ import "../interfaces/IWormhole.sol"; import "./GasOracleState.sol"; -abstract contract GasOracleGetters is GasOracleState { +contract GasOracleGetters is GasOracleState { function isInitialized(address implementation) public view returns (bool) { return _state.initializedImplementations[implementation]; } @@ -20,12 +20,12 @@ abstract contract GasOracleGetters is GasOracleState { return _state.provider.chainId; } - function gasPrice(uint16 targetChainId) public view returns (uint256) { - return _state.gasPrices[targetChainId]; + function gasPrice(uint16 targetChainId) public view returns (uint128) { + return _state.data[targetChainId].gasPrice; } - function nativeCurrencyPrice(uint16 targetChainId) public view returns (uint256) { - return _state.nativeCurrencyPrices[targetChainId]; + function nativeCurrencyPrice(uint16 targetChainId) public view returns (uint128) { + return _state.data[targetChainId].nativeCurrencyPrice; } function owner() public view returns (address) { diff --git a/ethereum/contracts/gasOracle/GasOracleGovernance.sol b/ethereum/contracts/gasOracle/GasOracleGovernance.sol deleted file mode 100644 index df5e627..0000000 --- a/ethereum/contracts/gasOracle/GasOracleGovernance.sol +++ /dev/null @@ -1,35 +0,0 @@ -// contracts/Oracle.sol -// SPDX-License-Identifier: Apache 2 - -pragma solidity ^0.8.0; - -import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol"; - -import "../libraries/external/BytesLib.sol"; - -import "./GasOracleGetters.sol"; -import "./GasOracleSetters.sol"; - -import "../interfaces/IWormhole.sol"; - -abstract contract GasOracleGovernance is GasOracleGetters, GasOracleSetters, ERC1967Upgrade { - event ContractUpgraded(address indexed oldContract, address indexed newContract); - - function upgradeImplementation(address newImplementation) public onlyOwner { - require(newImplementation != address(0), "newImplementation == address(0)"); - - _upgradeTo(newImplementation); - - // Call initialize function of the new implementation - (bool success, bytes memory reason) = newImplementation.delegatecall(abi.encodeWithSignature("initialize()")); - - require(success, string(reason)); - - emit ContractUpgraded(_getImplementation(), newImplementation); - } - - modifier onlyOwner() { - require(owner() == _msgSender(), "owner() != _msgSender()"); - _; - } -} diff --git a/ethereum/contracts/gasOracle/GasOracleImplementation.sol b/ethereum/contracts/gasOracle/GasOracleImplementation.sol deleted file mode 100644 index 08ce614..0000000 --- a/ethereum/contracts/gasOracle/GasOracleImplementation.sol +++ /dev/null @@ -1,29 +0,0 @@ -// contracts/Implementation.sol -// SPDX-License-Identifier: Apache 2 - -pragma solidity ^0.8.0; -pragma experimental ABIEncoderV2; - -import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol"; - -import "./GasOracle.sol"; - - -contract GasOracleImplementation is GasOracle { - function initialize() initializer public virtual { - // this function needs to be exposed for an upgrade to pass - } - - modifier initializer() { - address impl = ERC1967Upgrade._getImplementation(); - - require( - !isInitialized(impl), - "already initialized" - ); - - setInitialized(impl); - - _; - } -} diff --git a/ethereum/contracts/gasOracle/GasOracleProxy.sol b/ethereum/contracts/gasOracle/GasOracleProxy.sol deleted file mode 100644 index e700b99..0000000 --- a/ethereum/contracts/gasOracle/GasOracleProxy.sol +++ /dev/null @@ -1,15 +0,0 @@ -// contracts/Wormhole.sol -// SPDX-License-Identifier: Apache 2 - -pragma solidity ^0.8.0; - -import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; - -contract GasOracleProxy is ERC1967Proxy { - constructor (address implementation, bytes memory initData) - ERC1967Proxy( - implementation, - initData - ) - {} -} \ No newline at end of file diff --git a/ethereum/contracts/gasOracle/GasOracleSetters.sol b/ethereum/contracts/gasOracle/GasOracleSetters.sol index a6e54e1..0a36d36 100644 --- a/ethereum/contracts/gasOracle/GasOracleSetters.sol +++ b/ethereum/contracts/gasOracle/GasOracleSetters.sol @@ -7,7 +7,7 @@ import "@openzeppelin/contracts/utils/Context.sol"; import "./GasOracleState.sol"; -abstract contract GasOracleSetters is Context, GasOracleState { +contract GasOracleSetters is Context, GasOracleState { function setInitialized(address implementation) internal { _state.initializedImplementations[implementation] = true; } @@ -24,8 +24,8 @@ abstract contract GasOracleSetters is Context, GasOracleState { _state.owner = owner; } - function setPriceInfo(uint16 updateChainId, uint256 updateGasPrice, uint256 updateNativeCurrencyPrice) internal { - _state.gasPrices[updateChainId] = updateGasPrice; - _state.nativeCurrencyPrices[updateChainId] = updateNativeCurrencyPrice; + function setPriceInfo(uint16 updateChainId, uint128 updateGasPrice, uint128 updateNativeCurrencyPrice) internal { + _state.data[updateChainId].gasPrice = updateGasPrice; + _state.data[updateChainId].nativeCurrencyPrice = updateNativeCurrencyPrice; } } diff --git a/ethereum/contracts/gasOracle/GasOracleSetup.sol b/ethereum/contracts/gasOracle/GasOracleSetup.sol deleted file mode 100644 index 30bc1e5..0000000 --- a/ethereum/contracts/gasOracle/GasOracleSetup.sol +++ /dev/null @@ -1,29 +0,0 @@ -// contracts/Setup.sol -// SPDX-License-Identifier: Apache 2 - -pragma solidity ^0.8.0; - -pragma experimental ABIEncoderV2; - -import "./GasOracleGovernance.sol"; - -import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol"; - -contract GasOracleSetup is GasOracleSetters, ERC1967Upgrade { - function setup(address implementation, address owner, address wormhole, uint16 srcChainId) public { - setupInitialState(owner, wormhole, srcChainId); - - require(implementation != address(0), "implementation == address(0)"); - _upgradeTo(implementation); - } - - function setupInitialState(address owner, address wormhole, uint16 srcChainId) internal { - require(owner != address(0), "owner == address(0)"); - setOwner(owner); - require(srcChainId > 0, "srcChainId == 0"); - setChainId(srcChainId); - // might use this later to consume price data via VAAs? - require(wormhole != address(0), "wormhole == address(0)"); - setWormhole(wormhole); - } -} diff --git a/ethereum/contracts/gasOracle/GasOracleState.sol b/ethereum/contracts/gasOracle/GasOracleState.sol index 616da92..51a1179 100644 --- a/ethereum/contracts/gasOracle/GasOracleState.sol +++ b/ethereum/contracts/gasOracle/GasOracleState.sol @@ -3,21 +3,25 @@ pragma solidity ^0.8.0; -abstract contract GasOracleStorage { +contract GasOracleStorage { struct Provider { uint16 chainId; address payable wormhole; } + struct PriceData { + uint128 gasPrice; + uint128 nativeCurrencyPrice; + } + struct State { Provider provider; address owner; mapping(address => bool) initializedImplementations; - mapping(uint16 => uint256) gasPrices; - mapping(uint16 => uint256) nativeCurrencyPrices; + mapping(uint16 => PriceData) data; } } -abstract contract GasOracleState { +contract GasOracleState { GasOracleStorage.State _state; } diff --git a/ethereum/forge-test/GasOracle.t.sol b/ethereum/forge-test/GasOracle.t.sol index 7b90348..40e2608 100644 --- a/ethereum/forge-test/GasOracle.t.sol +++ b/ethereum/forge-test/GasOracle.t.sol @@ -3,112 +3,95 @@ pragma solidity ^0.8.0; import "../contracts/gasOracle/GasOracle.sol"; -import "../contracts/gasOracle/GasOracleSetup.sol"; +import "../contracts/gasOracle/GasOracleState.sol"; import "forge-std/Test.sol"; import "forge-std/console.sol"; -contract TestGasOracle is GasOracle, GasOracleSetup, Test { +contract TestGasOracle is Test { uint16 constant TEST_ORACLE_CHAIN_ID = 2; //uint256 constant TEST_SRC_GAS_PRICE = 10; //uint256 constant TEST_SRC_NATIVE_CURRENCY_PRICE = 250; - function initializeGasOracle(address oracleOwner, uint16 oracleChainId) internal { - setupInitialState( - oracleOwner, // owner + GasOracle internal gasOracle; + + function initializeGasOracle(uint16 oracleChainId) internal { + gasOracle = new GasOracle( 0xC89Ce4735882C9F0f0FE26686c53074E09B0D550, // wormhole oracleChainId // chainId ); } - function testSetupInitialState(address oracleOwner, address wormhole, uint16 srcChainId) public { - vm.assume(oracleOwner != address(0)); + function testSetupInitialState(address wormhole, uint16 srcChainId) public { vm.assume(wormhole != address(0)); vm.assume(srcChainId > 0); - setupInitialState( - oracleOwner, // owner + gasOracle = new GasOracle( wormhole, // wormhole srcChainId // srcChainId ); - require(owner() == oracleOwner, "owner() != expected"); - require(chainId() == srcChainId, "chainId() != expected"); - require(_state.provider.wormhole == wormhole, "_state.provider.wormhole != expected"); + require(gasOracle.owner() == address(this), "owner() != expected"); + require(gasOracle.chainId() == srcChainId, "chainId() != expected"); // TODO: check slots? } - function testCannotInitializeWithOwnerZeroAddress(uint16 srcChainId) public { - vm.assume(srcChainId > 0); - - // you shall not pass - vm.expectRevert("owner == address(0)"); - initializeGasOracle( - address(0), // owner - srcChainId // srcChainId - ); - } - function testCannotInitializeWithChainIdZero(address oracleOwner) public { vm.assume(oracleOwner != address(0)); // you shall not pass vm.expectRevert("srcChainId == 0"); initializeGasOracle( - oracleOwner, // owner 0 // srcChainId ); } - function testCannotUpdatePriceWithChainIdZero(uint256 updateGasPrice, uint256 updateNativeCurrencyPrice) public { + function testCannotUpdatePriceWithChainIdZero(uint128 updateGasPrice, uint128 updateNativeCurrencyPrice) public { vm.assume(updateGasPrice > 0); vm.assume(updateNativeCurrencyPrice > 0); initializeGasOracle( - _msgSender(), // owner TEST_ORACLE_CHAIN_ID // chainId ); // you shall not pass vm.expectRevert("updateChainId == 0"); - updatePrice( + gasOracle.updatePrice( 0, // updateChainId updateGasPrice, updateNativeCurrencyPrice ); } - function testCannotUpdatePriceWithGasPriceZero(uint16 updateChainId, uint256 updateNativeCurrencyPrice) public { + function testCannotUpdatePriceWithGasPriceZero(uint16 updateChainId, uint128 updateNativeCurrencyPrice) public { vm.assume(updateChainId > 0); vm.assume(updateNativeCurrencyPrice > 0); initializeGasOracle( - _msgSender(), // owner TEST_ORACLE_CHAIN_ID // chainId ); // you shall not pass vm.expectRevert("updateGasPrice == 0"); - updatePrice( + gasOracle.updatePrice( updateChainId, 0, // updateGasPrice == 0 updateNativeCurrencyPrice ); } - function testCannotUpdatePriceWithNativeCurrencyPriceZero(uint16 updateChainId, uint256 updateGasPrice) public { + function testCannotUpdatePriceWithNativeCurrencyPriceZero(uint16 updateChainId, uint128 updateGasPrice) public { vm.assume(updateChainId > 0); vm.assume(updateGasPrice > 0); initializeGasOracle( - _msgSender(), // owner TEST_ORACLE_CHAIN_ID // chainId ); // you shall not pass vm.expectRevert("updateNativeCurrencyPrice == 0"); - updatePrice( + gasOracle.updatePrice( updateChainId, updateGasPrice, 0 // updateNativeCurrencyPrice == 0 @@ -118,35 +101,31 @@ contract TestGasOracle is GasOracle, GasOracleSetup, Test { function testCanUpdatePriceOnlyAsOwner( address oracleOwner, uint16 updateChainId, - uint256 updateGasPrice, - uint256 updateNativeCurrencyPrice + uint128 updateGasPrice, + uint128 updateNativeCurrencyPrice ) public { vm.assume(oracleOwner != address(0)); - vm.assume(oracleOwner != _msgSender()); + vm.assume(oracleOwner != address(this)); vm.assume(updateChainId > 0); vm.assume(updateGasPrice > 0); vm.assume(updateNativeCurrencyPrice > 0); initializeGasOracle( - oracleOwner, TEST_ORACLE_CHAIN_ID // chainId ); // you shall not pass + vm.prank(oracleOwner); vm.expectRevert("owner() != _msgSender()"); - updatePrice( - updateChainId, // chainId - updateGasPrice, // gasPrice - updateNativeCurrencyPrice // nativeCurrencyPrice - ); + gasOracle.updatePrice(updateChainId, updateGasPrice, updateNativeCurrencyPrice); } function testCannotGetPriceBeforeUpdateSrcPrice( uint16 dstChainId, - uint256 dstGasPrice, - uint256 dstNativeCurrencyPrice + uint128 dstGasPrice, + uint128 dstNativeCurrencyPrice ) public { @@ -159,22 +138,21 @@ contract TestGasOracle is GasOracle, GasOracleSetup, Test { vm.assume(dstNativeCurrencyPrice < 2 ** 128); initializeGasOracle( - _msgSender(), TEST_ORACLE_CHAIN_ID // chainId ); // update the price with reasonable values - updatePrice(dstChainId, dstGasPrice, dstNativeCurrencyPrice); + gasOracle.updatePrice(dstChainId, dstGasPrice, dstNativeCurrencyPrice); // you shall not pass vm.expectRevert("srcNativeCurrencyPrice == 0"); - getPrice(dstChainId); + gasOracle.getPrice(dstChainId); } function testCannotGetPriceBeforeUpdateDstPrice( uint16 dstChainId, - uint256 srcGasPrice, - uint256 srcNativeCurrencyPrice + uint128 srcGasPrice, + uint128 srcNativeCurrencyPrice ) public { @@ -187,24 +165,28 @@ contract TestGasOracle is GasOracle, GasOracleSetup, Test { vm.assume(srcNativeCurrencyPrice < 2 ** 128); initializeGasOracle( - _msgSender(), TEST_ORACLE_CHAIN_ID // chainId ); + console.log("address(this)", address(this)); + console.log("msg.sender", msg.sender); + console.log("msg.sender", msg.sender); + console.log("gasOracle.owner()", gasOracle.owner()); // update the price with reasonable values - updatePrice(TEST_ORACLE_CHAIN_ID, srcGasPrice, srcNativeCurrencyPrice); + //vm.prank(gasOracle.owner()); + gasOracle.updatePrice(TEST_ORACLE_CHAIN_ID, srcGasPrice, srcNativeCurrencyPrice); // you shall not pass vm.expectRevert("dstNativeCurrencyPrice == 0"); - getPrice(dstChainId); + gasOracle.getPrice(dstChainId); } function testUpdatePrice( uint16 dstChainId, - uint256 dstGasPrice, - uint256 dstNativeCurrencyPrice, - uint256 srcGasPrice, - uint256 srcNativeCurrencyPrice + uint128 dstGasPrice, + uint128 dstNativeCurrencyPrice, + uint128 srcGasPrice, + uint128 srcNativeCurrencyPrice ) public { @@ -214,32 +196,26 @@ contract TestGasOracle is GasOracle, GasOracleSetup, Test { vm.assume(dstNativeCurrencyPrice > 0); vm.assume(srcGasPrice > 0); vm.assume(srcNativeCurrencyPrice > 0); - // we will also assume reasonable values for gasPrice and nativeCurrencyPrice - vm.assume(dstGasPrice < 2 ** 128); - vm.assume(dstNativeCurrencyPrice < 2 ** 128); - vm.assume(srcGasPrice < 2 ** 128); - vm.assume(srcNativeCurrencyPrice < 2 ** 128); initializeGasOracle( - _msgSender(), TEST_ORACLE_CHAIN_ID // chainId ); // update the prices with reasonable values - updatePrice(dstChainId, dstGasPrice, dstNativeCurrencyPrice); - updatePrice(TEST_ORACLE_CHAIN_ID, srcGasPrice, srcNativeCurrencyPrice); + gasOracle.updatePrice(dstChainId, dstGasPrice, dstNativeCurrencyPrice); + gasOracle.updatePrice(TEST_ORACLE_CHAIN_ID, srcGasPrice, srcNativeCurrencyPrice); // verify price - uint256 expected = dstGasPrice * dstNativeCurrencyPrice / srcNativeCurrencyPrice; - require(getPrice(dstChainId) == expected, "getPrice(updateChainId) != expected"); + uint256 expected = (uint256(dstGasPrice) * dstNativeCurrencyPrice) / srcNativeCurrencyPrice; + require(gasOracle.getPrice(dstChainId) == expected, "gasOracle.getPrice(updateChainId) != expected"); } function testUpdatePrices( uint16 dstChainId, - uint256 dstGasPrice, - uint256 dstNativeCurrencyPrice, - uint256 srcGasPrice, - uint256 srcNativeCurrencyPrice + uint128 dstGasPrice, + uint128 dstNativeCurrencyPrice, + uint128 srcGasPrice, + uint128 srcNativeCurrencyPrice ) public { @@ -249,31 +225,28 @@ contract TestGasOracle is GasOracle, GasOracleSetup, Test { vm.assume(dstNativeCurrencyPrice > 0); vm.assume(srcGasPrice > 0); vm.assume(srcNativeCurrencyPrice > 0); - // we will also assume reasonable values for gasPrice and nativeCurrencyPrice - vm.assume(dstGasPrice < 2 ** 128); - vm.assume(dstNativeCurrencyPrice < 2 ** 128); - vm.assume(srcGasPrice < 2 ** 128); - vm.assume(srcNativeCurrencyPrice < 2 ** 128); initializeGasOracle( - _msgSender(), TEST_ORACLE_CHAIN_ID // chainId ); - UpdatePrice[] memory updates = new UpdatePrice[](2); - updates[0] = UpdatePrice({ + GasOracle.UpdatePrice[] memory updates = new GasOracle.UpdatePrice[](2); + updates[0] = GasOracle.UpdatePrice({ chainId: TEST_ORACLE_CHAIN_ID, gasPrice: srcGasPrice, nativeCurrencyPrice: srcNativeCurrencyPrice }); - updates[1] = - UpdatePrice({chainId: dstChainId, gasPrice: dstGasPrice, nativeCurrencyPrice: dstNativeCurrencyPrice}); + updates[1] = GasOracle.UpdatePrice({ + chainId: dstChainId, + gasPrice: dstGasPrice, + nativeCurrencyPrice: dstNativeCurrencyPrice + }); // update the prices with reasonable values - updatePrices(updates); + gasOracle.updatePrices(updates); // verify price - uint256 expected = dstGasPrice * dstNativeCurrencyPrice / srcNativeCurrencyPrice; - require(getPrice(dstChainId) == expected, "getPrice(updateChainId) != expected"); + uint256 expected = (uint256(dstGasPrice) * dstNativeCurrencyPrice) / srcNativeCurrencyPrice; + require(gasOracle.getPrice(dstChainId) == expected, "gasOracle.getPrice(updateChainId) != expected"); } }