From 12b18d4ccc5f3f7ba6c5571d4faeab6c3d17bcbd Mon Sep 17 00:00:00 2001 From: derpy-duck <115193320+derpy-duck@users.noreply.github.com> Date: Wed, 14 Jun 2023 17:01:14 -0400 Subject: [PATCH] Relayer new delivery provider implementation (#3088) * WIP * Pricing Wallet test * Give owner same permissions as price oracle --- .../DeliveryProviderGetters.sol | 4 +++ .../DeliveryProviderGovernance.sol | 28 +++++++++++++------ .../DeliveryProviderSetters.sol | 4 +++ .../DeliveryProviderState.sol | 2 ++ .../forge-test/relayer/RelayProvider.t.sol | 27 ++++++++++++++---- 5 files changed, 51 insertions(+), 14 deletions(-) diff --git a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGetters.sol b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGetters.sol index 86229c34a..d26ca0a0b 100644 --- a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGetters.sol +++ b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGetters.sol @@ -17,6 +17,10 @@ contract DeliveryProviderGetters is DeliveryProviderState { return _state.pendingOwner; } + function pricingWallet() public view returns (address) { + return _state.pricingWallet; + } + function isInitialized(address impl) public view returns (bool) { return _state.initializedImplementations[impl]; } diff --git a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGovernance.sol b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGovernance.sol index dd33def40..b8e886e16 100644 --- a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGovernance.sol +++ b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderGovernance.sol @@ -31,6 +31,7 @@ abstract contract DeliveryProviderGovernance is error AddressIsZero(); error CallerMustBePendingOwner(); error CallerMustBeOwner(); + error CallerMustBeOwnerOrPricingWallet(); event ContractUpgraded(address indexed oldContract, address indexed newContract); event ChainSupportUpdated(uint16 targetChain, bool isSupported); @@ -69,6 +70,10 @@ abstract contract DeliveryProviderGovernance is } } + function updatePricingWallet(address newPricingWallet) external onlyOwner { + setPricingWallet(newPricingWallet); + } + function updateSupportedChainImpl(uint16 targetChain, bool isSupported) internal { setChainSupported(targetChain, isSupported); emit ChainSupportUpdated(targetChain, isSupported); @@ -106,13 +111,13 @@ abstract contract DeliveryProviderGovernance is emit TargetChainAddressUpdated(targetChain, newAddress); } - function updateDeliverGasOverhead(uint16 chainId, Gas newGasOverhead) external onlyOwner { + function updateDeliverGasOverhead(uint16 chainId, Gas newGasOverhead) external onlyOwnerOrPricingWallet { updateDeliverGasOverheadImpl(chainId, newGasOverhead); } function updateDeliverGasOverheads( DeliveryProviderStructs.DeliverGasOverheadUpdate[] memory overheadUpdates - ) external onlyOwner { + ) external onlyOwnerOrPricingWallet { uint256 updatesLength = overheadUpdates.length; for (uint256 i = 0; i < updatesLength;) { DeliveryProviderStructs.DeliverGasOverheadUpdate memory update = overheadUpdates[i]; @@ -133,13 +138,13 @@ abstract contract DeliveryProviderGovernance is uint16 updateChainId, GasPrice updateGasPrice, WeiPrice updateNativeCurrencyPrice - ) external onlyOwner { + ) external onlyOwnerOrPricingWallet { updatePriceImpl(updateChainId, updateGasPrice, updateNativeCurrencyPrice); } function updatePrices(DeliveryProviderStructs.UpdatePrice[] memory updates) external - onlyOwner + onlyOwnerOrPricingWallet { uint256 pricesLength = updates.length; for (uint256 i = 0; i < pricesLength;) { @@ -169,13 +174,13 @@ abstract contract DeliveryProviderGovernance is setPriceInfo(updateChainId, updateGasPrice, updateNativeCurrencyPrice); } - function updateMaximumBudget(uint16 targetChain, Wei maximumTotalBudget) external onlyOwner { + function updateMaximumBudget(uint16 targetChain, Wei maximumTotalBudget) external onlyOwnerOrPricingWallet { setMaximumBudget(targetChain, maximumTotalBudget); } function updateMaximumBudgets(DeliveryProviderStructs.MaximumBudgetUpdate[] memory updates) external - onlyOwner + onlyOwnerOrPricingWallet { uint256 updatesLength = updates.length; for (uint256 i = 0; i < updatesLength;) { @@ -191,13 +196,13 @@ abstract contract DeliveryProviderGovernance is uint16 targetChain, uint16 buffer, uint16 bufferDenominator - ) external onlyOwner { + ) external onlyOwnerOrPricingWallet { updateAssetConversionBufferImpl(targetChain, buffer, bufferDenominator); } function updateAssetConversionBuffers( DeliveryProviderStructs.AssetConversionBufferUpdate[] memory updates - ) external onlyOwner { + ) external onlyOwnerOrPricingWallet { uint256 updatesLength = updates.length; for (uint256 i = 0; i < updatesLength;) { DeliveryProviderStructs.AssetConversionBufferUpdate memory update = updates[i]; @@ -326,4 +331,11 @@ abstract contract DeliveryProviderGovernance is } _; } + + modifier onlyOwnerOrPricingWallet() { + if ((pricingWallet() != _msgSender()) && (owner() != _msgSender())) { + revert CallerMustBeOwnerOrPricingWallet(); + } + _; + } } diff --git a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderSetters.sol b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderSetters.sol index fd9b26b52..1f6f5bc66 100644 --- a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderSetters.sol +++ b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderSetters.sol @@ -28,6 +28,10 @@ contract DeliveryProviderSetters is Context, DeliveryProviderState { _state.chainId = thisChain; } + function setPricingWallet(address newPricingWallet) internal { + _state.pricingWallet = newPricingWallet; + } + function setWormholeRelayer(address payable coreRelayer) internal { _state.coreRelayer = coreRelayer; } diff --git a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderState.sol b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderState.sol index 2c1877c89..13e797865 100644 --- a/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderState.sol +++ b/ethereum/contracts/relayer/deliveryProvider/DeliveryProviderState.sol @@ -28,6 +28,8 @@ contract DeliveryProviderStorage { address owner; // Pending target of ownership transfer. address pendingOwner; + // Address that is allowed to modify pricing + address pricingWallet; // Address of the core relayer contract. address coreRelayer; // Dictionary of implementation contract -> initialized flag diff --git a/ethereum/forge-test/relayer/RelayProvider.t.sol b/ethereum/forge-test/relayer/RelayProvider.t.sol index 4cade8596..b4dc14e05 100644 --- a/ethereum/forge-test/relayer/RelayProvider.t.sol +++ b/ethereum/forge-test/relayer/RelayProvider.t.sol @@ -100,23 +100,36 @@ contract TestDeliveryProvider is Test { ); } - function testCanUpdatePriceOnlyAsOwner( - address oracleOwner, + function testCanUpdatePriceOnlyAsOwnerOrPriceWallet( + address pricingWallet, + address maliciousUser, uint16 updateChainId, GasPrice updateGasPrice, WeiPrice updateNativeCurrencyPrice ) public { - vm.assume(oracleOwner != address(0)); - vm.assume(oracleOwner != address(this)); + vm.assume(maliciousUser != address(0)); + vm.assume(pricingWallet != address(0)); + vm.assume(maliciousUser != pricingWallet); + vm.assume(maliciousUser != address(this)); vm.assume(updateChainId > 0); vm.assume(updateGasPrice.unwrap() > 0); vm.assume(updateNativeCurrencyPrice.unwrap() > 0); + vm.assume(updateGasPrice.unwrap() < type(uint64).max); + vm.assume(updateNativeCurrencyPrice.unwrap() < type(uint128).max); initializeDeliveryProvider(); + deliveryProvider.updatePricingWallet(pricingWallet); // you shall not pass - vm.prank(oracleOwner); - vm.expectRevert(abi.encodeWithSignature("CallerMustBeOwner()")); + vm.prank(maliciousUser); + vm.expectRevert(abi.encodeWithSignature("CallerMustBeOwnerOrPricingWallet()")); + deliveryProvider.updatePrice(updateChainId, updateGasPrice, updateNativeCurrencyPrice); + + // pricing wallet + vm.prank(pricingWallet); + deliveryProvider.updatePrice(updateChainId, updateGasPrice, updateNativeCurrencyPrice); + + // owner deliveryProvider.updatePrice(updateChainId, updateGasPrice, updateNativeCurrencyPrice); } @@ -289,6 +302,8 @@ contract TestDeliveryProvider is Test { vm.assume(gasOverhead < uint256(2)**31); + + // update the prices with reasonable values deliveryProvider.updatePrice( dstChainId, GasPrice.wrap(dstGasPrice), WeiPrice.wrap(dstNativeCurrencyPrice)