Skip to content

Commit

Permalink
LM-143-add-finance-role (#1040)
Browse files Browse the repository at this point in the history
## Motivation
Finance needs special permissions to move funds on the LM

## Solution
Create a special role for finance
  • Loading branch information
RyanRHall authored Jun 24, 2024
1 parent a9370e1 commit 2b4df02
Show file tree
Hide file tree
Showing 10 changed files with 283 additions and 43 deletions.
30 changes: 16 additions & 14 deletions contracts/gas-snapshots/liquiditymanager.gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
LiquidityManager__report:test_EmptyReportReverts() (gas: 11159)
LiquidityManager_addLiquidity:test_addLiquiditySuccess() (gas: 279154)
LiquidityManager_rebalanceLiquidity:test_InsufficientLiquidityReverts() (gas: 204896)
LiquidityManager_rebalanceLiquidity:test_InvalidRemoteChainReverts() (gas: 192069)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess() (gas: 8785965)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8719591)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8714799)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8642942)
LiquidityManager_rebalanceLiquidity:test_rebalanceLiquiditySuccess() (gas: 381075)
LiquidityManager_rebalanceLiquidity:test_InsufficientLiquidityReverts() (gas: 206745)
LiquidityManager_rebalanceLiquidity:test_InvalidRemoteChainReverts() (gas: 192319)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess() (gas: 9038370)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8847984)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8843190)
LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8770997)
LiquidityManager_rebalanceLiquidity:test_rebalanceLiquiditySuccess() (gas: 382907)
LiquidityManager_receive:test_receive_success() (gas: 21182)
LiquidityManager_removeLiquidity:test_InsufficientLiquidityReverts() (gas: 184602)
LiquidityManager_removeLiquidity:test_OnlyOwnerReverts() (gas: 10967)
LiquidityManager_removeLiquidity:test_removeLiquiditySuccess() (gas: 236075)
LiquidityManager_setCrossChainRebalancer:test_OnlyOwnerReverts() (gas: 17027)
LiquidityManager_removeLiquidity:test_InsufficientLiquidityReverts() (gas: 184852)
LiquidityManager_removeLiquidity:test_OnlyFinanceRoleReverts() (gas: 10850)
LiquidityManager_removeLiquidity:test_removeLiquiditySuccess() (gas: 236324)
LiquidityManager_setCrossChainRebalancer:test_OnlyOwnerReverts() (gas: 17005)
LiquidityManager_setCrossChainRebalancer:test_ZeroAddressReverts() (gas: 21630)
LiquidityManager_setCrossChainRebalancer:test_ZeroChainSelectorReverts() (gas: 13105)
LiquidityManager_setCrossChainRebalancer:test_setCrossChainRebalancerSuccess() (gas: 162208)
LiquidityManager_setCrossChainRebalancer:test_setCrossChainRebalancerSuccess() (gas: 162186)
LiquidityManager_setFinanceRole:test_OnlyOwnerReverts() (gas: 10987)
LiquidityManager_setFinanceRole:test_setFinanceRoleSuccess() (gas: 21702)
LiquidityManager_setLocalLiquidityContainer:test_OnlyOwnerReverts() (gas: 11030)
LiquidityManager_setLocalLiquidityContainer:test_ReverstWhen_CalledWithTheZeroAddress() (gas: 10621)
LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3437694)
LiquidityManager_setMinimumLiquidity:test_OnlyOwnerReverts() (gas: 10925)
LiquidityManager_setMinimumLiquidity:test_setMinimumLiquiditySuccess() (gas: 36434)
LiquidityManager_withdrawNative:test_OnlyOwnerReverts() (gas: 13115)
LiquidityManager_withdrawNative:test_withdrawNative_success() (gas: 51041)
LiquidityManager_withdrawNative:test_OnlyFinanceRoleReverts() (gas: 13046)
LiquidityManager_withdrawNative:test_withdrawNative_success() (gas: 51398)
OCR3Base_setOCR3Config:testFMustBePositiveReverts() (gas: 12245)
OCR3Base_setOCR3Config:testFTooHighReverts() (gas: 12429)
OCR3Base_setOCR3Config:testOracleOutOfRegisterReverts() (gas: 14847)
Expand Down
37 changes: 32 additions & 5 deletions contracts/src/v0.8/liquiditymanager/LiquidityManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
error InsufficientLiquidity(uint256 requested, uint256 available, uint256 reserve);
error EmptyReport();
error TransferFailed();
error OnlyFinanceRole();

/// @notice Emitted when a finalization step is completed without funds being available.
/// @param ocrSeqNum The OCR sequence number of the report.
Expand All @@ -42,6 +43,10 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
bytes bridgeSpecificData
);

/// @notice Emitted when the CLL finance role is set.
/// @param financeRole The address of the new finance role.
event FinanceRoleSet(address financeRole);

/// @notice Emitted when liquidity is transferred to another chain, or received from another chain.
/// @param ocrSeqNum The OCR sequence number of the report.
/// @param fromChainSelector The chain selector of the chain the funds are coming from.
Expand Down Expand Up @@ -149,11 +154,15 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
/// @dev In the case of CCIP, this would be the token pool.
ILiquidityContainer private s_localLiquidityContainer;

/// @notice The CLL finance team multisig
address private s_finance;

constructor(
IERC20 token,
uint64 localChainSelector,
ILiquidityContainer localLiquidityContainer,
uint256 minimumLiquidity
uint256 minimumLiquidity,
address finance
) OCR3Base() {
if (localChainSelector == 0) {
revert ZeroChainSelector();
Expand All @@ -166,6 +175,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
i_localChainSelector = localChainSelector;
s_localLiquidityContainer = localLiquidityContainer;
s_minimumLiquidity = minimumLiquidity;
s_finance = finance;
}

// ================================================================
Expand All @@ -177,7 +187,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
}

/// @notice withdraw native balance
function withdrawNative(uint256 amount, address payable destination) external onlyOwner {
function withdrawNative(uint256 amount, address payable destination) external onlyFinance {
(bool success, ) = destination.call{value: amount}("");
if (!success) revert TransferFailed();

Expand Down Expand Up @@ -212,7 +222,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {

/// @notice Removes liquidity from the system and sends it to the caller, so the owner.
/// @dev Only the owner can call this function.
function removeLiquidity(uint256 amount) external onlyOwner {
function removeLiquidity(uint256 amount) external onlyFinance {
uint256 currentBalance = getLiquidity();
if (currentBalance < amount) {
revert InsufficientLiquidity(amount, currentBalance, 0);
Expand All @@ -232,7 +242,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
uint256 amount,
uint256 nativeBridgeFee,
bytes calldata bridgeSpecificPayload
) external onlyOwner {
) external onlyFinance {
_rebalanceLiquidity(chainSelector, amount, nativeBridgeFee, type(uint64).max, bridgeSpecificPayload);
}

Expand All @@ -244,7 +254,7 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
uint256 amount,
bool shouldWrapNative,
bytes calldata bridgeSpecificPayload
) external onlyOwner {
) external onlyFinance {
_receiveLiquidity(remoteChainSelector, amount, bridgeSpecificPayload, shouldWrapNative, type(uint64).max);
}

Expand Down Expand Up @@ -537,4 +547,21 @@ contract LiquidityManager is ILiquidityManager, OCR3Base {
s_minimumLiquidity = minimumLiquidity;
emit MinimumLiquiditySet(oldLiquidity, s_minimumLiquidity);
}

/// @notice Gets the CLL finance team multisig address
function getFinanceRole() external view returns (address) {
return s_finance;
}

/// @notice Sets the finance team multisig address
/// @dev Only the owner can call this function.
function setFinanceRole(address finance) external onlyOwner {
s_finance = finance;
emit FinanceRoleSet(finance);
}

modifier onlyFinance() {
if (msg.sender != s_finance) revert OnlyFinanceRole();
_;
}
}
66 changes: 52 additions & 14 deletions contracts/src/v0.8/liquiditymanager/test/LiquidityManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ contract LiquidityManagerSetup is LiquidityManagerBaseTest {
bytes bridgeSpecificData,
bytes reason
);
event FinanceRoleSet(address financeRole);
event LiquidityAddedToContainer(address indexed provider, uint256 indexed amount);
event LiquidityRemovedFromContainer(address indexed remover, uint256 indexed amount);
// Liquidity container event
Expand All @@ -57,7 +58,13 @@ contract LiquidityManagerSetup is LiquidityManagerBaseTest {

s_bridgeAdapter = new MockL1BridgeAdapter(s_l1Token, false);
s_lockReleaseTokenPool = new LockReleaseTokenPool(s_l1Token, new address[](0), address(1), true, address(123));
s_liquidityManager = new LiquidityManagerHelper(s_l1Token, i_localChainSelector, s_lockReleaseTokenPool, 0);
s_liquidityManager = new LiquidityManagerHelper(
s_l1Token,
i_localChainSelector,
s_lockReleaseTokenPool,
0,
FINANCE
);

s_lockReleaseTokenPool.setRebalancer(address(s_liquidityManager));

Expand All @@ -73,7 +80,8 @@ contract LiquidityManagerSetup is LiquidityManagerBaseTest {
IERC20(address(s_l1Weth)),
i_localChainSelector,
s_wethLockReleaseTokenPool,
0
0,
FINANCE
);

s_wethLockReleaseTokenPool.setRebalancer(address(s_wethRebalancer));
Expand Down Expand Up @@ -105,8 +113,9 @@ contract LiquidityManager_removeLiquidity is LiquidityManagerSetup {
deal(address(s_l1Token), address(s_lockReleaseTokenPool), amount);

vm.expectEmit();
emit LiquidityRemovedFromContainer(OWNER, amount);
emit LiquidityRemovedFromContainer(FINANCE, amount);

vm.startPrank(FINANCE);
s_liquidityManager.removeLiquidity(amount);

assertEq(s_l1Token.balanceOf(address(s_liquidityManager)), 0);
Expand All @@ -120,13 +129,14 @@ contract LiquidityManager_removeLiquidity is LiquidityManagerSetup {

vm.expectRevert(abi.encodeWithSelector(LiquidityManager.InsufficientLiquidity.selector, requested, balance, 0));

vm.startPrank(FINANCE);
s_liquidityManager.removeLiquidity(requested);
}

function test_OnlyOwnerReverts() external {
function test_OnlyFinanceRoleReverts() external {
vm.stopPrank();

vm.expectRevert("Only callable by owner");
vm.expectRevert(LiquidityManager.OnlyFinanceRole.selector);

s_liquidityManager.removeLiquidity(123);
}
Expand Down Expand Up @@ -182,6 +192,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
encodedNonce
);

vm.startPrank(FINANCE);
s_liquidityManager.rebalanceLiquidity(i_remoteChainSelector, AMOUNT, 0, bytes(""));

assertEq(s_l1Token.balanceOf(address(s_liquidityManager)), 0);
Expand All @@ -196,14 +207,15 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
function test_rebalanceBetweenPoolsSuccess() external {
uint256 amount = 12345670;

s_liquidityManager = new LiquidityManagerHelper(s_l1Token, i_localChainSelector, s_bridgeAdapter, 0);
s_liquidityManager = new LiquidityManagerHelper(s_l1Token, i_localChainSelector, s_bridgeAdapter, 0, FINANCE);

MockL1BridgeAdapter mockRemoteBridgeAdapter = new MockL1BridgeAdapter(s_l1Token, false);
LiquidityManager mockRemoteRebalancer = new LiquidityManager(
s_l1Token,
i_remoteChainSelector,
mockRemoteBridgeAdapter,
0
0,
FINANCE
);

LiquidityManager.CrossChainRebalancerArgs[] memory args = new LiquidityManager.CrossChainRebalancerArgs[](1);
Expand All @@ -229,6 +241,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {

deal(address(s_l1Token), address(s_bridgeAdapter), amount);

vm.startPrank(FINANCE);
s_liquidityManager.rebalanceLiquidity(i_remoteChainSelector, amount, 0, bytes(""));

assertEq(s_l1Token.balanceOf(address(s_bridgeAdapter)), 0);
Expand Down Expand Up @@ -264,7 +277,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
true,
address(123)
);
LiquidityManager remoteRebalancer = new LiquidityManager(s_l2Token, i_remoteChainSelector, remotePool, 0);
LiquidityManager remoteRebalancer = new LiquidityManager(s_l2Token, i_remoteChainSelector, remotePool, 0, FINANCE);

// set rebalancer role on the pool.
remotePool.setRebalancer(address(remoteRebalancer));
Expand Down Expand Up @@ -314,6 +327,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
bridgeSpecificPayload,
bridgeSendReturnData
);
vm.startPrank(FINANCE);
remoteRebalancer.rebalanceLiquidity(i_localChainSelector, AMOUNT, 0, bridgeSpecificPayload);

// available liquidity has been moved to the remote bridge adapter from the token pool.
Expand Down Expand Up @@ -392,7 +406,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
true,
address(123)
);
LiquidityManager remoteRebalancer = new LiquidityManager(s_l2Token, i_remoteChainSelector, remotePool, 0);
LiquidityManager remoteRebalancer = new LiquidityManager(s_l2Token, i_remoteChainSelector, remotePool, 0, FINANCE);

// set rebalancer role on the pool.
remotePool.setRebalancer(address(remoteRebalancer));
Expand Down Expand Up @@ -443,6 +457,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
bridgeSpecificPayload,
bridgeSendReturnData
);
vm.startPrank(FINANCE);
remoteRebalancer.rebalanceLiquidity(i_localChainSelector, AMOUNT, 0, bridgeSpecificPayload);

// available liquidity has been moved to the remote bridge adapter from the token pool.
Expand Down Expand Up @@ -516,7 +531,8 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
IERC20(address(s_l2Weth)),
i_remoteChainSelector,
remotePool,
0
0,
FINANCE
);

// set rebalancer role on the pool.
Expand Down Expand Up @@ -557,8 +573,8 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
s_l2Weth.deposit{value: AMOUNT}();
vm.stopPrank();

// switch back to owner for the rest of the test to avoid reverts.
vm.startPrank(OWNER);
// switch to finance for the rest of the test to avoid reverts.
vm.startPrank(FINANCE);

// initiate a send from remote rebalancer to s_wethRebalancer.
uint256 nonce = 1;
Expand Down Expand Up @@ -645,6 +661,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {
deal(address(s_l1Token), address(s_lockReleaseTokenPool), AMOUNT);
vm.expectRevert(abi.encodeWithSelector(LiquidityManager.InsufficientLiquidity.selector, AMOUNT, AMOUNT, 3));

vm.startPrank(FINANCE);
s_liquidityManager.rebalanceLiquidity(0, AMOUNT, 0, bytes(""));
}

Expand All @@ -653,6 +670,7 @@ contract LiquidityManager_rebalanceLiquidity is LiquidityManagerSetup {

vm.expectRevert(abi.encodeWithSelector(LiquidityManager.InvalidRemoteChain.selector, i_remoteChainSelector));

vm.startPrank(FINANCE);
s_liquidityManager.rebalanceLiquidity(i_remoteChainSelector, AMOUNT, 0, bytes(""));
}
}
Expand Down Expand Up @@ -844,6 +862,25 @@ contract LiquidityManager_setMinimumLiquidity is LiquidityManagerSetup {
}
}

contract LiquidityManager_setFinanceRole is LiquidityManagerSetup {
event MinimumLiquiditySet(uint256 oldBalance, uint256 newBalance);

function test_setFinanceRoleSuccess() external {
vm.expectEmit();
address newFinanceRole = makeAddr("newFinanceRole");
assertEq(s_liquidityManager.getFinanceRole(), FINANCE);
emit FinanceRoleSet(newFinanceRole);
s_liquidityManager.setFinanceRole(newFinanceRole);
assertEq(s_liquidityManager.getFinanceRole(), newFinanceRole);
}

function test_OnlyOwnerReverts() external {
vm.stopPrank();
vm.expectRevert("Only callable by owner");
s_liquidityManager.setFinanceRole(address(1));
}
}

contract LiquidityManager_withdrawNative is LiquidityManagerSetup {
event NativeWithdrawn(uint256 amount, address destination);

Expand All @@ -858,13 +895,14 @@ contract LiquidityManager_withdrawNative is LiquidityManagerSetup {
assertEq(receiver.balance, 0);
vm.expectEmit();
emit NativeWithdrawn(1, receiver);
vm.startPrank(FINANCE);
s_liquidityManager.withdrawNative(1, payable(receiver));
assertEq(receiver.balance, 1);
}

function test_OnlyOwnerReverts() external {
function test_OnlyFinanceRoleReverts() external {
vm.stopPrank();
vm.expectRevert("Only callable by owner");
vm.expectRevert(LiquidityManager.OnlyFinanceRole.selector);
s_liquidityManager.withdrawNative(1, payable(receiver));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ contract LiquidityManagerBaseTest is Test {
uint64 internal immutable i_localChainSelector = 1234;
uint64 internal immutable i_remoteChainSelector = 9876;

address internal constant FINANCE = address(0x00000fffffffffffffffffffff);
address internal constant OWNER = address(0x00000078772732723782873283);
address internal constant STRANGER = address(0x00000999999911111111222222);

Expand All @@ -32,7 +33,9 @@ contract LiquidityManagerBaseTest is Test {
s_l2Weth = new WETH9();

vm.startPrank(OWNER);
vm.label(OWNER, "Owner");
vm.label(STRANGER, "Stranger");

vm.label(FINANCE, "FINANCE");
vm.label(OWNER, "OWNER");
vm.label(STRANGER, "STRANGER");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ contract LiquidityManagerHelper is LiquidityManager {
IERC20 token,
uint64 localChainSelector,
ILiquidityContainer localLiquidityContainer,
uint256 targetTokens
) LiquidityManager(token, localChainSelector, localLiquidityContainer, targetTokens) {}
uint256 targetTokens,
address finance
) LiquidityManager(token, localChainSelector, localLiquidityContainer, targetTokens, finance) {}

function report(bytes calldata rep, uint64 ocrSeqNum) external {
_report(rep, ocrSeqNum);
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ arbitrum_token_gateway: ../../../contracts/solc/v0.8.24/IArbitrumTokenGateway/IA
arbsys: ../../../contracts/solc/v0.8.24/IArbSys/IArbSys.abi ../../../contracts/solc/v0.8.24/IArbSys/IArbSys.bin 70adb49f157d8e077485d1a5c87ddf64b214822aef736bb68e122a77bab78a16
l2_arbitrum_gateway: ../../../contracts/solc/v0.8.24/IL2ArbitrumGateway/IL2ArbitrumGateway.abi ../../../contracts/solc/v0.8.24/IL2ArbitrumGateway/IL2ArbitrumGateway.bin 4d1af2bdb0aeb0b15e3cbc4ed2158f9bcba4925f68ed3669e0ecfde14115c893
l2_arbitrum_messenger: ../../../contracts/solc/v0.8.24/IL2ArbitrumMessenger/IL2ArbitrumMessenger.abi ../../../contracts/solc/v0.8.24/IL2ArbitrumMessenger/IL2ArbitrumMessenger.bin 84d4bfedf16e92e3fb15880832fa54a3a21808dffea8a7c0946cde3b5e17a0c3
liquiditymanager: ../../../contracts/solc/v0.8.24/LiquidityManager/LiquidityManager.abi ../../../contracts/solc/v0.8.24/LiquidityManager/LiquidityManager.bin 0165aba21e535ebdafb732c1ff9cebafa73a7fce65ea7b364a2cc6f55dc36156
liquiditymanager: ../../../contracts/solc/v0.8.24/LiquidityManager/LiquidityManager.abi ../../../contracts/solc/v0.8.24/LiquidityManager/LiquidityManager.bin e261f435bba025db25b03202c4cba8a87bbc3a2e50158497969fb696402a63e3
mock_l1_bridge_adapter: ../../../contracts/solc/v0.8.24/MockBridgeAdapter/MockL1BridgeAdapter.abi ../../../contracts/solc/v0.8.24/MockBridgeAdapter/MockL1BridgeAdapter.bin 538d2e3855031bcb4ef28ab8f0c54c8249e90936a588cde81b965d1dd2d08ad4
mock_l2_bridge_adapter: ../../../contracts/solc/v0.8.24/MockBridgeAdapter/MockL2BridgeAdapter.abi ../../../contracts/solc/v0.8.24/MockBridgeAdapter/MockL2BridgeAdapter.bin 8ff182e2ac6aac98e1fe85c37d6d92a0b0570de695ed1292127ae25babe96bda
no_op_ocr3: ../../../contracts/solc/v0.8.24/NoOpOCR3/NoOpOCR3.abi ../../../contracts/solc/v0.8.24/NoOpOCR3/NoOpOCR3.bin 964010f2f2c2b1f78d1149c19e6656fb50c8b76ff160b7d2b7e93e5446d19e3d
Expand Down
Loading

0 comments on commit 2b4df02

Please sign in to comment.