diff --git a/Makefile b/Makefile index dfc8845e4..3bc53220d 100644 --- a/Makefile +++ b/Makefile @@ -121,7 +121,7 @@ build-sizes: ## Builds the project and shows sizes .PHONY: test-vvv test-vvv: ## Runs tests with verbose output - forge test --match-contract SDMVW0TokenInputNoSlippageAMB1323 --evm-version cancun -vvv + forge test --match-test test_crossChainRebalance_updateSuperformData_allErrors --evm-version cancun -vvv .PHONY: ftest ftest: ## Runs tests with cancun evm version diff --git a/src/interfaces/ISuperformRouterPlusAsync.sol b/src/interfaces/ISuperformRouterPlusAsync.sol index 94842aac4..fe9f7bc5e 100644 --- a/src/interfaces/ISuperformRouterPlusAsync.sol +++ b/src/interfaces/ISuperformRouterPlusAsync.sol @@ -18,6 +18,9 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus { /// @notice thrown if the caller is not router plus error NOT_ROUTER_PLUS(); + /// @notice thrown if the caller is not core state registry rescuer + error NOT_CORE_STATE_REGISTRY_RESCUER(); + /// @notice thrown if the rebalance to update is invalid error COMPLETE_REBALANCE_INVALID_TX_DATA_UPDATE(); @@ -44,17 +47,17 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus { /// @notice thrown to avoid processing the same rebalance payload twice error REBALANCE_ALREADY_PROCESSED(); - /// @notice thrown when the refund proposer is invalid - error INVALID_PROPOSER(); + /// @notice thrown when the refund requester is not the payload receiver + error INVALID_REQUESTER(); /// @notice thrown when the refund payload is invalid error INVALID_REFUND_DATA(); - /// @notice thrown when refund is already proposed - error REFUND_ALREADY_PROPOSED(); + /// @notice thrown when requested refund amount is too high + error REQUESTED_AMOUNT_TOO_HIGH(); - /// @notice thrown if the refund is still in dispute phase - error IN_DISPUTE_PHASE(); + /// @notice thrown when the refund payload is already approved + error REFUND_ALREADY_APPROVED(); ////////////////////////////////////////////////////////////// // EVENTS // @@ -74,6 +77,15 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus { uint256 indexed routerPlusPayloadId, address indexed refundReceiver, address refundToken, uint256 refundAmount ); + /// @notice emitted when a refund is proposed + /// @param routerPlusPayloadId is the unique identifier for the payload + /// @param refundReceiver is the address of the user who'll receiver the refund + /// @param refundToken is the token to be refunded + /// @param refundAmount is the new refund amount + event refundRequested( + uint256 indexed routerPlusPayloadId, address indexed refundReceiver, address refundToken, uint256 refundAmount + ); + /// @notice emitted when an existing refund got disputed /// @param routerPlusPayloadId is the unique identifier for the payload /// @param disputer is the address of the user who disputed the refund @@ -97,7 +109,6 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus { address receiver; address interimToken; uint256 amount; - uint256 proposedTime; } struct DecodedRouterPlusRebalanceCallData { @@ -168,16 +179,12 @@ interface ISuperformRouterPlusAsync is IBaseSuperformRouterPlus { payable returns (bool rebalanceSuccessful); - /// @notice allows the receiver / disputer to protect against malicious processors - /// @param finalPayloadId_ is the unique identifier of the refund - function disputeRefund(uint256 finalPayloadId_) external; - - /// @notice allows the rescuer to propose a new refund amount after a successful dispute - /// @param finalPayloadId_ is the unique identifier of the refund - /// @param refundAmount_ is the new refund amount proposed - function proposeRefund(uint256 finalPayloadId_, uint256 refundAmount_) external; + /// @notice allows the user to request a refund for the rebalance + /// @param routerplusPayloadId_ the router plus payload id + function requestRefund(uint256 routerplusPayloadId_, uint256 requestedAmount) external; - /// @notice allows the user to claim their refund post the dispute period - /// @param finalPayloadId_ is the unique identifier of the refund - function finalizeRefund(uint256 finalPayloadId_) external; -} + /// @dev only callable by core state registry rescuer + /// @notice approves a refund for the rebalance and sends funds to the receiver + /// @param routerplusPayloadId_ the router plus payload id + function approveRefund(uint256 routerplusPayloadId_) external; +} \ No newline at end of file diff --git a/src/router-plus/SuperformRouterPlusAsync.sol b/src/router-plus/SuperformRouterPlusAsync.sol index 550c1cab2..5caa5fcde 100644 --- a/src/router-plus/SuperformRouterPlusAsync.sol +++ b/src/router-plus/SuperformRouterPlusAsync.sol @@ -35,6 +35,9 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou mapping(uint256 routerPlusPayloadId => Refund) public refunds; mapping(uint256 routerPlusPayloadId => bool processed) public processedRebalancePayload; + + mapping(uint256 routerPlusPayloadId => bool approvedRefund) public approvedRefund; + ////////////////////////////////////////////////////////////// // MODIFIERS // ////////////////////////////////////////////////////////////// @@ -53,6 +56,13 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou _; } + modifier onlyCoreStateRegistryRescuer() { + if (!_hasRole(keccak256("CORE_STATE_REGISTRY_RESCUER_ROLE"), msg.sender)) { + revert NOT_CORE_STATE_REGISTRY_RESCUER(); + } + _; + } + ////////////////////////////////////////////////////////////// // CONSTRUCTOR // ////////////////////////////////////////////////////////////// @@ -253,8 +263,8 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou ENTIRE_SLIPPAGE * args_.amountReceivedInterimAsset < ((data.expectedAmountInterimAsset * (ENTIRE_SLIPPAGE - data.slippage))) ) { - refunds[args_.routerPlusPayloadId] = - Refund(args_.receiverAddressSP, data.interimAsset, args_.amountReceivedInterimAsset, block.timestamp); + + refunds[args_.routerPlusPayloadId] = Refund(args_.receiverAddressSP, data.interimAsset, 0); emit RefundInitiated( args_.routerPlusPayloadId, args_.receiverAddressSP, data.interimAsset, args_.amountReceivedInterimAsset @@ -415,42 +425,30 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou } /// @inheritdoc ISuperformRouterPlusAsync - function disputeRefund(uint256 routerPlusPayloadId_) external override { - Refund storage r = refunds[routerPlusPayloadId_]; - - if (!(msg.sender == r.receiver || _hasRole(keccak256("CORE_STATE_REGISTRY_DISPUTER_ROLE"), msg.sender))) { - revert Error.NOT_VALID_DISPUTER(); - } - - if (r.proposedTime == 0 || block.timestamp > r.proposedTime + _getDelay()) revert Error.DISPUTE_TIME_ELAPSED(); - - /// @dev just can reset the last proposed time, since amounts should be updated again to - /// pass the proposedTime zero check in finalize - r.proposedTime = 0; - - emit RefundDisputed(routerPlusPayloadId_, msg.sender); - } + function requestRefund(uint256 routerPlusPayloadId_, uint256 requestedAmount) external { + Refund memory r = refunds[routerPlusPayloadId_]; - /// @inheritdoc ISuperformRouterPlusAsync - function proposeRefund(uint256 routerPlusPayloadId_, uint256 refundAmount_) external { - if (!_hasRole(keccak256("CORE_STATE_REGISTRY_RESCUER_ROLE"), msg.sender)) revert INVALID_PROPOSER(); + if (msg.sender != r.receiver) revert INVALID_REQUESTER(); + if (r.interimToken == address(0)) revert INVALID_REFUND_DATA(); - Refund storage r = refunds[routerPlusPayloadId_]; + XChainRebalanceData memory data = xChainRebalanceCallData[r.receiver][routerPlusPayloadId_]; - if (r.interimToken == address(0) || r.receiver == address(0)) revert INVALID_REFUND_DATA(); - if (r.proposedTime != 0) revert REFUND_ALREADY_PROPOSED(); + if (requestedAmount > data.expectedAmountInterimAsset) { + revert REQUESTED_AMOUNT_TOO_HIGH(); + } - r.proposedTime = block.timestamp; - r.amount = refundAmount_; + refunds[routerPlusPayloadId_] = Refund(msg.sender, data.interimAsset, requestedAmount); - emit NewRefundAmountProposed(routerPlusPayloadId_, refundAmount_); + emit refundRequested(routerPlusPayloadId_, msg.sender, r.interimToken, requestedAmount); } /// @inheritdoc ISuperformRouterPlusAsync - function finalizeRefund(uint256 routerPlusPayloadId_) external { + function approveRefund(uint256 routerPlusPayloadId_) external onlyCoreStateRegistryRescuer { + if (approvedRefund[routerPlusPayloadId_]) revert REFUND_ALREADY_APPROVED(); + Refund memory r = refunds[routerPlusPayloadId_]; - if (r.proposedTime == 0 || block.timestamp <= r.proposedTime + _getDelay()) revert IN_DISPUTE_PHASE(); + approvedRefund[routerPlusPayloadId_] = true; /// @dev deleting to prevent re-entrancy delete refunds[routerPlusPayloadId_]; @@ -464,15 +462,6 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou // INTERNAL FUNCTIONS // ////////////////////////////////////////////////////////////// - /// @dev returns the current dispute delay - function _getDelay() internal view returns (uint256) { - uint256 delay = superRegistry.delay(); - if (delay == 0) { - revert Error.DELAY_NOT_SET(); - } - return delay; - } - function _updateSuperformData( MultiVaultSFData memory sfData, LiqRequest[] memory liqRequests, diff --git a/test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol b/test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol index 0558f08bd..9d5eb1436 100644 --- a/test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol +++ b/test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol @@ -107,21 +107,23 @@ contract HyperlaneImplementationTest is CommonProtocolActions { vm.startPrank(deployer); uint256 userIndex = userSeed_ % users.length; + + vm.assume(malice_ != getContract(ETH, "CoreStateRegistry")); + vm.assume(malice_ != getContract(ETH, "TimelockStateRegistry")); + vm.assume(malice_ != getContract(ETH, "BroadcastRegistry")); + vm.assume(malice_ != getContract(ETH, "AsyncStateRegistry")); + AMBMessage memory ambMessage; BroadCastAMBExtraData memory ambExtraData; address coreStateRegistry; (ambMessage, ambExtraData, coreStateRegistry) = setupBroadcastPayloadAMBData(users[userIndex], address(hyperlaneImplementation)); - - vm.expectRevert(Error.NOT_STATE_REGISTRY.selector); - vm.assume(malice_ != getContract(ETH, "CoreStateRegistry")); - vm.assume(malice_ != getContract(ETH, "TimelockStateRegistry")); - vm.assume(malice_ != getContract(ETH, "BroadcastRegistry")); - vm.assume(malice_ != getContract(ETH, "AsyncStateRegistry")); + vm.stopPrank(); vm.deal(malice_, 100 ether); vm.prank(malice_); + vm.expectRevert(Error.NOT_STATE_REGISTRY.selector); hyperlaneImplementation.dispatchPayload{ value: 0.1 ether }( users[userIndex], chainIds[5], abi.encode(ambMessage), abi.encode(ambExtraData) ); diff --git a/test/mainnet/SmokeTest.Staging.t.sol b/test/mainnet/SmokeTest.Staging.t.sol index ec8c49af2..46ea05041 100644 --- a/test/mainnet/SmokeTest.Staging.t.sol +++ b/test/mainnet/SmokeTest.Staging.t.sol @@ -555,14 +555,13 @@ contract SmokeTestStaging is MainnetBaseSetup { for (uint256 i; i < TARGET_DEPLOYMENT_CHAINS.length; ++i) { uint64 chainId = TARGET_DEPLOYMENT_CHAINS[i]; - vm.selectFork(FORKS[chainId]); superFactory = SuperformFactory(getContract(chainId, "SuperformFactory")); chainId != BLAST ? assertEq(superFactory.getFormImplementation(5), getContract(chainId, "ERC5115Form")) : assertEq(superFactory.getFormImplementation(205), getContract(chainId, "ERC5115Form")); - /// @dev in blast there are 6 forms (2 without operator, 2 with wrong state registry and 2 right superforms) + assertEq(superFactory.getFormCount(), chainId == LINEA ? 5 : chainId == BLAST ? 8 : 7); assertEq(superFactory.getFormStateRegistryId(5), 1); } diff --git a/test/unit/payments/PaymentHelper.t.sol b/test/unit/payments/PaymentHelper.t.sol index bba518bb0..503aea624 100644 --- a/test/unit/payments/PaymentHelper.t.sol +++ b/test/unit/payments/PaymentHelper.t.sol @@ -569,7 +569,6 @@ contract PaymentHelperTest is ProtocolActions { assertEq(fees3, 0); } - function test_estimateSingleDirectMultiVault() public view { /// @dev scenario: single vault withdrawal diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index 35dcb6a50..8694aeed3 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -1698,56 +1698,65 @@ contract SuperformRouterPlusTest is ProtocolActions { SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).completeCrossChainRebalance{ value: 1 ether }(completeArgs); vm.stopPrank(); - vm.expectRevert(Error.NOT_VALID_DISPUTER.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).disputeRefund(1); + // Step 5: Request refund - vm.startPrank(deployer); - vm.mockCall( - address(getContract(SOURCE_CHAIN, "SuperRegistry")), - abi.encodeWithSelector(ISuperRegistry.delay.selector), - abi.encode(0) - ); - vm.expectRevert(Error.DELAY_NOT_SET.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).disputeRefund(1); - vm.clearMockedCalls(); - - vm.warp(block.timestamp + 100 days); - vm.expectRevert(Error.DISPUTE_TIME_ELAPSED.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).disputeRefund(1); - - vm.warp(block.timestamp - 100 days); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).disputeRefund(1); + /// @dev testing invalid requester (not receiver) + vm.startPrank(address(222)); + vm.expectRevert(ISuperformRouterPlusAsync.INVALID_REQUESTER.selector); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).requestRefund(1, 100); + vm.stopPrank(); - vm.expectRevert(Error.DISPUTE_TIME_ELAPSED.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).disputeRefund(1); + // @dev testing refund amount exceeds expected amount + vm.startPrank(deployer); + vm.expectRevert(ISuperformRouterPlusAsync.REQUESTED_AMOUNT_TOO_HIGH.selector); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).requestRefund(1, 1000e18); vm.stopPrank(); - vm.expectRevert(ISuperformRouterPlusAsync.INVALID_PROPOSER.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).proposeRefund(1, completeArgs.amountReceivedInterimAsset); + /// @dev testing valid refund request + vm.prank(deployer); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).requestRefund(1, 100); - vm.startPrank(deployer); - vm.expectRevert(ISuperformRouterPlusAsync.INVALID_REFUND_DATA.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).proposeRefund(2, completeArgs.amountReceivedInterimAsset); + (,, uint256 requestedAmount) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + assertEq(requestedAmount, 100); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).proposeRefund(1, completeArgs.amountReceivedInterimAsset); + (, address refundToken,) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + assertEq(refundToken, address(args.interimAsset)); - vm.expectRevert(ISuperformRouterPlusAsync.REFUND_ALREADY_PROPOSED.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).proposeRefund(1, completeArgs.amountReceivedInterimAsset); + // Step 6: Approve refund - vm.expectRevert(ISuperformRouterPlusAsync.IN_DISPUTE_PHASE.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).finalizeRefund(1); + /// @dev testing invalid approver (not core state registry) + vm.startPrank(address(1234)); + vm.expectRevert(ISuperformRouterPlusAsync.NOT_CORE_STATE_REGISTRY_RESCUER.selector); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).approveRefund(1); + vm.stopPrank(); - (, address refundToken,,) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + /// @dev testing valid refund approval uint256 balanceBefore = MockERC20(refundToken).balanceOf(deployer); + uint256 routerBalanceBefore = MockERC20(refundToken).balanceOf(address(ROUTER_PLUS_ASYNC_SOURCE)); + vm.startPrank(deployer); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).approveRefund(1); + vm.stopPrank(); - vm.warp(block.timestamp + 100 days); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).finalizeRefund(1); uint256 balanceAfter = MockERC20(refundToken).balanceOf(deployer); - assertGt(balanceAfter, balanceBefore); + assertEq(MockERC20(refundToken).balanceOf(address(ROUTER_PLUS_ASYNC_SOURCE)), routerBalanceBefore - 100); + assertEq(MockERC20(refundToken).balanceOf(address(deployer)), balanceBefore + 100); + + (, address interimToken,) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + assertEq(interimToken, address(0)); + + (, address receiver,) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + assertEq(receiver, address(0)); - vm.expectRevert(ISuperformRouterPlusAsync.IN_DISPUTE_PHASE.selector); - SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).finalizeRefund(1); + (,, uint256 updatedRequestedAmount) = SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).refunds(1); + assertEq(updatedRequestedAmount, 0); + vm.stopPrank(); + + /// @dev testing refund already approved + vm.startPrank(deployer); + vm.expectRevert(ISuperformRouterPlusAsync.REFUND_ALREADY_APPROVED.selector); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).approveRefund(1); + vm.stopPrank(); } function test_crossChainRebalance_negativeSlippage() public { @@ -1804,6 +1813,7 @@ contract SuperformRouterPlusTest is ProtocolActions { function test_crossChainRebalance_updateSuperformData_allErrors() public { vm.selectFork(FORKS[SOURCE_CHAIN]); + SingleVaultSFData memory sfData = SingleVaultSFData({ superformId: superformId1, amount: 1e18, @@ -1911,14 +1921,19 @@ contract SuperformRouterPlusTest is ProtocolActions { // Reset liqDstChainId completeArgs.liqRequests[0][0].liqDstChainId = SOURCE_CHAIN; - - /// @FIXME: This line is not reacheable - // vm.expectRevert(ISuperformRouterPlusAsync.COMPLETE_REBALANCE_DIFFERENT_RECEIVER.selector); - // completeArgs.receiverAddressSP = address(0x123); - // SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).completeCrossChainRebalance{ value: 1 ether - // }(completeArgs); vm.stopPrank(); + vm.startPrank(ROUTER_PLUS_SOURCE); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).setXChainRebalanceCallData(address(0x123), 2, data); + vm.stopPrank(); + vm.startPrank(deployer); + vm.expectRevert(ISuperformRouterPlusAsync.COMPLETE_REBALANCE_DIFFERENT_RECEIVER.selector); + completeArgs.routerPlusPayloadId = 2; + completeArgs.receiverAddressSP = address(0x123); + SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).completeCrossChainRebalance{ value: 1 ether }(completeArgs); + vm.stopPrank(); + completeArgs.routerPlusPayloadId = 1; + completeArgs.receiverAddressSP = deployer; sfData.liqRequest.token = address(0); data = IBaseSuperformRouterPlus.XChainRebalanceData({ @@ -1939,6 +1954,7 @@ contract SuperformRouterPlusTest is ProtocolActions { completeArgs.routerPlusPayloadId = 2; vm.expectRevert(ISuperformRouterPlusAsync.COMPLETE_REBALANCE_INVALID_TX_DATA_UPDATE.selector); SuperformRouterPlusAsync(ROUTER_PLUS_ASYNC_SOURCE).completeCrossChainRebalance{ value: 1 ether }(completeArgs); + vm.stopPrank(); } //////////////////////////////////////////////////////////////