Skip to content

Commit 15d5530

Browse files
0xTimepunk0xTimepunk
authored
Merge pull request #643 from superform-xyz/tamara-sup-8878-fix-approvals-in-routerplus
fix: update approvals in routerplus [SUP-8878]
2 parents 19cbbd1 + ccc551a commit 15d5530

File tree

9 files changed

+830
-83
lines changed

9 files changed

+830
-83
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ build-sizes: ## Builds the project and shows sizes
121121

122122
.PHONY: test-vvv
123123
test-vvv: ## Runs tests with verbose output
124-
forge test --match-test test_crossChainRebalance_updateSuperformData_allErrors --evm-version cancun -vvv
124+
forge test --match-test test_rebalanceMultiPositions_tokenRefunds_interimDust_allowanceNot0 --evm-version cancun -vvvvv
125125

126126
.PHONY: ftest
127127
ftest: ## Runs tests with cancun evm version

‎src/interfaces/ISuperformRouterPlus.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,15 @@ interface ISuperformRouterPlus is IBaseSuperformRouterPlus {
4949
/// @notice thrown if the amount of assets received is lower than the slippage
5050
error ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
5151

52+
/// @notice thrown if the slippage is invalid
53+
error INVALID_GLOBAL_SLIPPAGE();
54+
5255
/// @notice thrown if the tolerance is exceeded during shares redemption
5356
error TOLERANCE_EXCEEDED();
5457

58+
/// @notice thrown if the amountIn is not equal or lower than the balance available
59+
error AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();
60+
5561
//////////////////////////////////////////////////////////////
5662
// EVENTS //
5763
//////////////////////////////////////////////////////////////
@@ -230,4 +236,9 @@ interface ISuperformRouterPlus is IBaseSuperformRouterPlus {
230236
/// @dev Forwards dust to Paymaster
231237
/// @param token_ the token to forward
232238
function forwardDustToPaymaster(address token_) external;
239+
240+
/// @dev only callable by Emergency Admin
241+
/// @notice sets the global slippage for all rebalances
242+
/// @param slippage_ The slippage tolerance for same chain rebalances
243+
function setGlobalSlippage(uint256 slippage_) external;
233244
}

‎src/router-plus/BaseSuperformRouterPlus.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { IERC1155Receiver } from "openzeppelin-contracts/contracts/token/ERC1155
66
import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/IERC165.sol";
77
import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol";
88
import { SafeERC20 } from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
9+
import { ISuperRBAC } from "src/interfaces/ISuperRBAC.sol";
910

1011
import { ISuperRegistry } from "src/interfaces/ISuperRegistry.sol";
1112
import { IBaseSuperformRouterPlus } from "src/interfaces/IBaseSuperformRouterPlus.sol";
@@ -143,4 +144,12 @@ abstract contract BaseSuperformRouterPlus is IBaseSuperformRouterPlus, IERC1155R
143144
function _getAddress(bytes32 id_) internal view returns (address) {
144145
return superRegistry.getAddress(id_);
145146
}
147+
148+
/// @dev returns if an address has a specific role
149+
/// @param id_ the role id
150+
/// @param addressToCheck_ the address to check
151+
/// @return true if the address has the role, false otherwise
152+
function _hasRole(bytes32 id_, address addressToCheck_) internal view returns (bool) {
153+
return ISuperRBAC(superRegistry.getAddress(keccak256("SUPER_RBAC"))).hasRole(id_, addressToCheck_);
154+
}
146155
}

‎src/router-plus/SuperformRouterPlus.sol

Lines changed: 115 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,16 @@ import {
1818
import { IBaseRouter } from "src/interfaces/IBaseRouter.sol";
1919
import { ISuperformRouterPlus, IERC20 } from "src/interfaces/ISuperformRouterPlus.sol";
2020
import { ISuperformRouterPlusAsync } from "src/interfaces/ISuperformRouterPlusAsync.sol";
21+
import { LiqRequest } from "src/types/DataTypes.sol";
22+
import { IBridgeValidator } from "src/interfaces/IBridgeValidator.sol";
2123

2224
/// @title SuperformRouterPlus
2325
/// @dev Performs rebalances and deposits on the Superform platform
2426
/// @author Zeropoint Labs
2527
contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
2628
using SafeERC20 for IERC20;
2729

30+
uint256 public GLOBAL_SLIPPAGE;
2831
uint256 public ROUTER_PLUS_PAYLOAD_ID;
2932

3033
/// @dev Tolerance constant to account for tokens with rounding issues on transfer
@@ -34,7 +37,10 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
3437
// CONSTRUCTOR //
3538
//////////////////////////////////////////////////////////////
3639

37-
constructor(address superRegistry_) BaseSuperformRouterPlus(superRegistry_) { }
40+
constructor(address superRegistry_) BaseSuperformRouterPlus(superRegistry_) {
41+
/// @dev default to 0.1% slippage as a start
42+
GLOBAL_SLIPPAGE = 10;
43+
}
3844

3945
//////////////////////////////////////////////////////////////
4046
// EXTERNAL WRITE FUNCTIONS //
@@ -357,9 +363,8 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
357363

358364
/// @inheritdoc ISuperformRouterPlus
359365
function deposit4626(address[] calldata vaults_, Deposit4626Args[] calldata args) external payable {
360-
361366
uint256 length = vaults_.length;
362-
367+
363368
if (length != args.length) {
364369
revert Error.ARRAY_LENGTH_MISMATCH();
365370
}
@@ -390,6 +395,19 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
390395
}
391396
}
392397

398+
/// @inheritdoc ISuperformRouterPlus
399+
function setGlobalSlippage(uint256 slippage_) external {
400+
if (!_hasRole(keccak256("EMERGENCY_ADMIN_ROLE"), msg.sender)) {
401+
revert Error.NOT_PRIVILEGED_CALLER(keccak256("EMERGENCY_ADMIN_ROLE"));
402+
}
403+
404+
if (slippage_ > ENTIRE_SLIPPAGE || slippage_ == 0) {
405+
revert INVALID_GLOBAL_SLIPPAGE();
406+
}
407+
408+
GLOBAL_SLIPPAGE = slippage_;
409+
}
410+
393411
//////////////////////////////////////////////////////////////
394412
// INTERNAL FUNCTIONS //
395413
//////////////////////////////////////////////////////////////
@@ -439,6 +457,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
439457
if (req.superformData.liqRequests[i].liqDstChainId != CHAIN_ID) {
440458
revert REBALANCE_MULTI_POSITIONS_DIFFERENT_CHAIN();
441459
}
460+
442461
if (req.superformData.amounts[i] != args.sharesToRedeem[i]) {
443462
revert REBALANCE_MULTI_POSITIONS_DIFFERENT_AMOUNTS();
444463
}
@@ -450,23 +469,29 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
450469
/// @dev send SPs to router
451470
_callSuperformRouter(router_, callData, args.rebalanceFromMsgValue);
452471

453-
uint256 amountToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;
472+
uint256 availableBalanceToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;
454473

455-
if (amountToDeposit == 0) revert Error.ZERO_AMOUNT();
474+
if (availableBalanceToDeposit == 0) revert Error.ZERO_AMOUNT();
456475

457476
if (
458-
ENTIRE_SLIPPAGE * amountToDeposit
477+
ENTIRE_SLIPPAGE * availableBalanceToDeposit
459478
< ((args.expectedAmountToReceivePostRebalanceFrom * (ENTIRE_SLIPPAGE - args.slippage)))
460479
) {
461480
revert Error.VAULT_IMPLEMENTATION_FAILED();
462481
}
463482

464-
/// @dev step 3: rebalance into a new superform with rebalanceCallData
465-
if (!whitelistedSelectors[Actions.DEPOSIT][_parseSelectorMem(rebalanceToCallData)]) {
466-
revert INVALID_DEPOSIT_SELECTOR();
467-
}
483+
uint256 amountIn = _validateAndGetAmountIn(rebalanceToCallData, availableBalanceToDeposit);
484+
485+
_deposit(router_, interimAsset, amountIn, args.rebalanceToMsgValue, rebalanceToCallData);
486+
}
468487

469-
_deposit(router_, interimAsset, amountToDeposit, args.rebalanceToMsgValue, rebalanceToCallData);
488+
function _takeAmountIn(LiqRequest memory liqReq, uint256 sfDataAmount) internal view returns (uint256 amountIn) {
489+
bytes memory txData = liqReq.txData;
490+
if (txData.length == 0) {
491+
amountIn = sfDataAmount;
492+
} else {
493+
amountIn = IBridgeValidator(superRegistry.getBridgeValidator(liqReq.bridgeId)).decodeAmountIn(txData, false);
494+
}
470495
}
471496

472497
function _transferSuperPositions(
@@ -523,9 +548,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
523548
if (assets < TOLERANCE_CONSTANT || balanceDifference < assets - TOLERANCE_CONSTANT) revert TOLERANCE_EXCEEDED();
524549

525550
/// @dev validate the slippage
526-
if (
527-
(ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_))))
528-
) {
551+
if ((ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_))))) {
529552
revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
530553
}
531554
}
@@ -604,7 +627,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
604627
/// @notice deposits ERC4626 vault shares into superform
605628
/// @param vault_ The ERC4626 vault to redeem from
606629
/// @param args Rest of the arguments to deposit 4626
607-
function _deposit4626(address vault_, Deposit4626Args calldata args, uint256 arrayLength) internal {
630+
function _deposit4626(address vault_, Deposit4626Args calldata args, uint256 arrayLength) internal {
608631
_transferERC20In(IERC20(vault_), args.receiverAddressSP, args.amount);
609632
IERC4626 vault = IERC4626(vault_);
610633
address assetAdr = vault.asset();
@@ -614,12 +637,88 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
614637

615638
uint256 amountRedeemed = _redeemShare(vault, assetAdr, args.amount, args.expectedOutputAmount, args.maxSlippage);
616639

640+
uint256 amountIn = _validateAndGetAmountIn(args.depositCallData, amountRedeemed);
641+
617642
uint256 msgValue = msg.value / arrayLength;
618643
address router = _getAddress(keccak256("SUPERFORM_ROUTER"));
619-
_deposit(router, asset, amountRedeemed, msgValue, args.depositCallData);
644+
645+
_deposit(router, asset, amountIn, msgValue, args.depositCallData);
620646

621647
_tokenRefunds(router, assetAdr, args.receiverAddressSP, balanceBefore);
622648

623649
emit Deposit4626Completed(args.receiverAddressSP, vault_);
624650
}
651+
652+
function _validateAndGetAmountIn(
653+
bytes calldata rebalanceToCallData,
654+
uint256 availableBalanceToDeposit
655+
)
656+
internal
657+
view
658+
returns (uint256 amountIn)
659+
{
660+
bytes4 rebalanceToSelector = _parseSelectorMem(rebalanceToCallData);
661+
662+
if (!whitelistedSelectors[Actions.DEPOSIT][rebalanceToSelector]) {
663+
revert INVALID_DEPOSIT_SELECTOR();
664+
}
665+
666+
uint256 amountInTemp;
667+
668+
if (rebalanceToSelector == IBaseRouter.singleDirectSingleVaultDeposit.selector) {
669+
SingleVaultSFData memory sfData =
670+
abi.decode(_parseCallData(rebalanceToCallData), (SingleDirectSingleVaultStateReq)).superformData;
671+
amountIn = _takeAmountIn(sfData.liqRequest, sfData.amount);
672+
} else if (rebalanceToSelector == IBaseRouter.singleXChainSingleVaultDeposit.selector) {
673+
SingleVaultSFData memory sfData =
674+
abi.decode(_parseCallData(rebalanceToCallData), (SingleXChainSingleVaultStateReq)).superformData;
675+
amountIn = _takeAmountIn(sfData.liqRequest, sfData.amount);
676+
} else if (rebalanceToSelector == IBaseRouter.singleDirectMultiVaultDeposit.selector) {
677+
MultiVaultSFData memory sfData =
678+
abi.decode(_parseCallData(rebalanceToCallData), (SingleDirectMultiVaultStateReq)).superformData;
679+
uint256 len = sfData.liqRequests.length;
680+
681+
for (uint256 i; i < len; ++i) {
682+
amountInTemp = _takeAmountIn(sfData.liqRequests[i], sfData.amounts[i]);
683+
amountIn += amountInTemp;
684+
}
685+
} else if (rebalanceToSelector == IBaseRouter.singleXChainMultiVaultDeposit.selector) {
686+
MultiVaultSFData memory sfData =
687+
abi.decode(_parseCallData(rebalanceToCallData), (SingleXChainMultiVaultStateReq)).superformsData;
688+
uint256 len = sfData.liqRequests.length;
689+
for (uint256 i; i < len; ++i) {
690+
amountInTemp = _takeAmountIn(sfData.liqRequests[i], sfData.amounts[i]);
691+
amountIn += amountInTemp;
692+
}
693+
} else if (rebalanceToSelector == IBaseRouter.multiDstSingleVaultDeposit.selector) {
694+
SingleVaultSFData[] memory sfData =
695+
abi.decode(_parseCallData(rebalanceToCallData), (MultiDstSingleVaultStateReq)).superformsData;
696+
uint256 lenDst = sfData.length;
697+
for (uint256 i; i < lenDst; ++i) {
698+
amountInTemp = _takeAmountIn(sfData[i].liqRequest, sfData[i].amount);
699+
amountIn += amountInTemp;
700+
}
701+
} else if (rebalanceToSelector == IBaseRouter.multiDstMultiVaultDeposit.selector) {
702+
MultiVaultSFData[] memory sfData =
703+
abi.decode(_parseCallData(rebalanceToCallData), (MultiDstMultiVaultStateReq)).superformsData;
704+
uint256 lenDst = sfData.length;
705+
for (uint256 i; i < lenDst; ++i) {
706+
uint256 len = sfData[i].liqRequests.length;
707+
for (uint256 j; j < len; ++j) {
708+
amountInTemp = _takeAmountIn(sfData[i].liqRequests[j], sfData[i].amounts[j]);
709+
amountIn += amountInTemp;
710+
}
711+
}
712+
}
713+
714+
/// @dev amountIn must be artificially off-chain reduced to be less than availableBalanceToDeposit otherwise the
715+
/// @dev approval to transfer tokens to SuperformRouter won't work
716+
if (amountIn > availableBalanceToDeposit) revert AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();
717+
718+
/// @dev check amountIn against availableBalanceToDeposit (available balance) via a GLOBAL_SLIPPAGE to prevent a
719+
/// @dev malicious keeper from sending a low amountIn
720+
if (ENTIRE_SLIPPAGE * amountIn < ((availableBalanceToDeposit * (ENTIRE_SLIPPAGE - GLOBAL_SLIPPAGE)))) {
721+
revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
722+
}
723+
}
625724
}

‎src/router-plus/SuperformRouterPlusAsync.sol

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ import {
1717
} from "src/router-plus/BaseSuperformRouterPlus.sol";
1818
import { IBaseRouter } from "src/interfaces/IBaseRouter.sol";
1919
import { ISuperformRouterPlusAsync } from "src/interfaces/ISuperformRouterPlusAsync.sol";
20-
import { ISuperRBAC } from "src/interfaces/ISuperRBAC.sol";
20+
import { SuperformFactory } from "src/SuperformFactory.sol";
21+
2122

2223
/// @title SuperformRouterPlusAsync
2324
/// @dev Completes the async step of cross chain rebalances and separates the balance from SuperformRouterPlus
@@ -527,12 +528,6 @@ contract SuperformRouterPlusAsync is ISuperformRouterPlusAsync, BaseSuperformRou
527528
return sfData;
528529
}
529530

530-
/// @dev returns if an address has a specific role
531-
532-
function _hasRole(bytes32 id_, address addressToCheck_) internal view returns (bool) {
533-
return ISuperRBAC(superRegistry.getAddress(keccak256("SUPER_RBAC"))).hasRole(id_, addressToCheck_);
534-
}
535-
536531
function _castToMultiVaultData(SingleVaultSFData memory data_)
537532
internal
538533
pure

‎test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,17 @@ contract HyperlaneImplementationTest is CommonProtocolActions {
107107
vm.startPrank(deployer);
108108
uint256 userIndex = userSeed_ % users.length;
109109

110-
111-
vm.assume(malice_ != getContract(ETH, "CoreStateRegistry"));
112-
vm.assume(malice_ != getContract(ETH, "TimelockStateRegistry"));
113-
vm.assume(malice_ != getContract(ETH, "BroadcastRegistry"));
114-
vm.assume(malice_ != getContract(ETH, "AsyncStateRegistry"));
115-
116110
AMBMessage memory ambMessage;
117111
BroadCastAMBExtraData memory ambExtraData;
118112
address coreStateRegistry;
119113

120114
(ambMessage, ambExtraData, coreStateRegistry) =
121115
setupBroadcastPayloadAMBData(users[userIndex], address(hyperlaneImplementation));
122-
vm.stopPrank();
116+
117+
vm.assume(malice_ != getContract(ETH, "CoreStateRegistry"));
118+
vm.assume(malice_ != getContract(ETH, "TimelockStateRegistry"));
119+
vm.assume(malice_ != getContract(ETH, "BroadcastRegistry"));
120+
vm.assume(malice_ != getContract(ETH, "AsyncStateRegistry"));
123121

124122
vm.deal(malice_, 100 ether);
125123
vm.prank(malice_);

‎test/mainnet/SmokeTest.Staging.t.sol

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,8 +568,6 @@ contract SmokeTestStaging is MainnetBaseSetup {
568568
}
569569

570570
function test_asyncStateRegistry() public {
571-
AsyncStateRegistry asyncStateRegistry;
572-
573571
for (uint256 i; i < TARGET_DEPLOYMENT_CHAINS.length; ++i) {
574572
uint64 chainId = TARGET_DEPLOYMENT_CHAINS[i];
575573
vm.selectFork(FORKS[chainId]);

0 commit comments

Comments
 (0)