Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update approvals in routerplus [SUP-8878] #643

Merged
merged 46 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ad7a5f5
feat: add setGlobalSlippage()
TamaraRingas Oct 17, 2024
b78b78e
feat: add _hasRole()
TamaraRingas Oct 17, 2024
8ebe706
feat: add setGlobalSlippage()
TamaraRingas Oct 17, 2024
f2c4ceb
refactor: decode amountIn in rebalanceSinglePosition()
TamaraRingas Oct 18, 2024
8a23ca2
fix: add slippage check in rebalanceSinglePosition()
TamaraRingas Oct 18, 2024
ea9d03e
fix: update approval amount in _transferSuperPositions()
TamaraRingas Oct 18, 2024
369bf3e
fix: move global slippage check to _rebalancePositionsSync()
TamaraRingas Oct 19, 2024
9ead37a
feat: test setGlobalSlippage()
TamaraRingas Oct 21, 2024
af6b2f5
refactor: add additional slippage checks to _rebalancePositionsSync()
TamaraRingas Oct 21, 2024
b912b3c
test: fix amount in calculation
0xTimepunk Oct 21, 2024
c8af790
Merge pull request #644 from superform-xyz/tamara-sup-8878-joao-fix
TamaraRingas Oct 22, 2024
b8036d3
refactor: rm global slippage setter in constructor
TamaraRingas Oct 22, 2024
25193dc
refactor: set global slippage in setUp()
TamaraRingas Oct 22, 2024
c398275
reorder test_revert_dispatchPayload_invalidCaller() to revert correctly
TamaraRingas Oct 22, 2024
9a4c463
chore: update test_rebalanceSinglePosition_multiDstMultiVaultDepositS…
TamaraRingas Oct 25, 2024
9f02765
chore: update test_rebalanceSinglePosition_multiDstMultiVaultDepositS…
TamaraRingas Oct 25, 2024
9817472
fix: simplify test structure
0xTimepunk Oct 25, 2024
bb0867a
fix: rm tests relating to disputeRefund()
TamaraRingas Oct 26, 2024
260f2a1
fix: rm overrides
TamaraRingas Oct 26, 2024
e246f49
fix: update liqReqs for multidstmultivault rebalance
TamaraRingas Oct 26, 2024
c9f1be4
chore: add test_rebalanceSinglePosition_singleXChainMultiVaultDeposit()
TamaraRingas Oct 28, 2024
7885dfc
chore: add test_rebalanceSinglePosition_singleXChainSingleVaultDeposi…
TamaraRingas Oct 28, 2024
55422db
chore: add _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
71b836a
fix: update superformIds in test_rebalanceSinglePosition_multiDstSing…
TamaraRingas Oct 28, 2024
751d71c
fix: update return storage location _buildRebalanceMultiMultiDstSingl…
TamaraRingas Oct 28, 2024
7be0cc8
fix: update _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
e1f91fc
fix: update _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
79fe34d
refactor: split _buildRebalanceMultiMultiDstSingleVaultArgs() into he…
TamaraRingas Oct 28, 2024
4c79081
add _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
c8a3e8a
fix: update _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
fa32bf5
fix: update _xChainDeposit() calls in test_rebalanceSinglePosition_mu…
TamaraRingas Oct 28, 2024
a775456
fix: update params in _buildSingleVaultSFData()
TamaraRingas Oct 28, 2024
ca0e563
update REBALANCE_TO chainId in test_rebalanceSinglePosition_multiDstS…
TamaraRingas Oct 28, 2024
8e2c0b7
fix: update REBALANCE_TO chainId in test_rebalanceSinglePosition_mult…
TamaraRingas Oct 28, 2024
2d7208a
fix: update fork in test_rebalanceSinglePosition_multiDstSingleVaultD…
TamaraRingas Oct 28, 2024
9e60021
fix: rm unused helper functions
TamaraRingas Oct 28, 2024
53d46c0
fux: rm _buildRebalanceMultiMultiDstSingleVaultArgs()
TamaraRingas Oct 28, 2024
b613612
fix: update test_rebalanceMultiPositions_multiDstSingleVaultDepositSe…
TamaraRingas Oct 29, 2024
61a12ab
import MultiDstSingleVaultStateReq
TamaraRingas Oct 29, 2024
4fcfde5
test: fix rebalanceMultiPositions_multiDstSingleVaultDepositSelector
0xTimepunk Oct 29, 2024
bdb146f
test: fix test_rebalanceMultiPositions_multiDstMultiVaultDepositSelector
0xTimepunk Oct 29, 2024
572bc8b
fix: remaining issues
0xTimepunk Oct 29, 2024
1f74c88
Merge remote-tracking branch 'origin/v1.5' into tamara-sup-8878-fix-a…
0xTimepunk Oct 29, 2024
0d7ea69
fix: internalize approvals fix and add to deposit4626
0xTimepunk Oct 30, 2024
86e76c5
fix: slippage check in wrong order
0xTimepunk Oct 30, 2024
ccc551a
fix: balanceDiff
0xTimepunk Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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-test test_crossChainRebalance_updateSuperformData_allErrors --evm-version cancun -vvv
forge test --match-test test_rebalanceMultiPositions_tokenRefunds_interimDust_allowanceNot0 --evm-version cancun -vvvvv

.PHONY: ftest
ftest: ## Runs tests with cancun evm version
Expand Down
11 changes: 11 additions & 0 deletions src/interfaces/ISuperformRouterPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,15 @@ interface ISuperformRouterPlus is IBaseSuperformRouterPlus {
/// @notice thrown if the amount of assets received is lower than the slippage
error ASSETS_RECEIVED_OUT_OF_SLIPPAGE();

/// @notice thrown if the slippage is invalid
error INVALID_GLOBAL_SLIPPAGE();

/// @notice thrown if the tolerance is exceeded during shares redemption
error TOLERANCE_EXCEEDED();

/// @notice thrown if the amountIn is not equal or lower than the balance available
error AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();

//////////////////////////////////////////////////////////////
// EVENTS //
//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -230,4 +236,9 @@ interface ISuperformRouterPlus is IBaseSuperformRouterPlus {
/// @dev Forwards dust to Paymaster
/// @param token_ the token to forward
function forwardDustToPaymaster(address token_) external;

/// @dev only callable by Emergency Admin
/// @notice sets the global slippage for all rebalances
/// @param slippage_ The slippage tolerance for same chain rebalances
function setGlobalSlippage(uint256 slippage_) external;
}
9 changes: 9 additions & 0 deletions src/router-plus/BaseSuperformRouterPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { IERC1155Receiver } from "openzeppelin-contracts/contracts/token/ERC1155
import { IERC165 } from "openzeppelin-contracts/contracts/utils/introspection/IERC165.sol";
import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol";
import { SafeERC20 } from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import { ISuperRBAC } from "src/interfaces/ISuperRBAC.sol";

import { ISuperRegistry } from "src/interfaces/ISuperRegistry.sol";
import { IBaseSuperformRouterPlus } from "src/interfaces/IBaseSuperformRouterPlus.sol";
Expand Down Expand Up @@ -143,4 +144,12 @@ abstract contract BaseSuperformRouterPlus is IBaseSuperformRouterPlus, IERC1155R
function _getAddress(bytes32 id_) internal view returns (address) {
return superRegistry.getAddress(id_);
}

/// @dev returns if an address has a specific role
/// @param id_ the role id
/// @param addressToCheck_ the address to check
/// @return true if the address has the role, false otherwise
function _hasRole(bytes32 id_, address addressToCheck_) internal view returns (bool) {
return ISuperRBAC(superRegistry.getAddress(keccak256("SUPER_RBAC"))).hasRole(id_, addressToCheck_);
}
}
131 changes: 115 additions & 16 deletions src/router-plus/SuperformRouterPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ import {
import { IBaseRouter } from "src/interfaces/IBaseRouter.sol";
import { ISuperformRouterPlus, IERC20 } from "src/interfaces/ISuperformRouterPlus.sol";
import { ISuperformRouterPlusAsync } from "src/interfaces/ISuperformRouterPlusAsync.sol";
import { LiqRequest } from "src/types/DataTypes.sol";
import { IBridgeValidator } from "src/interfaces/IBridgeValidator.sol";

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

uint256 public GLOBAL_SLIPPAGE;
uint256 public ROUTER_PLUS_PAYLOAD_ID;

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

constructor(address superRegistry_) BaseSuperformRouterPlus(superRegistry_) { }
constructor(address superRegistry_) BaseSuperformRouterPlus(superRegistry_) {
/// @dev default to 0.1% slippage as a start
GLOBAL_SLIPPAGE = 10;
0xTimepunk marked this conversation as resolved.
Show resolved Hide resolved
}

//////////////////////////////////////////////////////////////
// EXTERNAL WRITE FUNCTIONS //
Expand Down Expand Up @@ -357,9 +363,8 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {

/// @inheritdoc ISuperformRouterPlus
function deposit4626(address[] calldata vaults_, Deposit4626Args[] calldata args) external payable {

uint256 length = vaults_.length;

if (length != args.length) {
revert Error.ARRAY_LENGTH_MISMATCH();
}
Expand Down Expand Up @@ -390,6 +395,19 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
}
}

/// @inheritdoc ISuperformRouterPlus
function setGlobalSlippage(uint256 slippage_) external {
if (!_hasRole(keccak256("EMERGENCY_ADMIN_ROLE"), msg.sender)) {
revert Error.NOT_PRIVILEGED_CALLER(keccak256("EMERGENCY_ADMIN_ROLE"));
}

if (slippage_ > ENTIRE_SLIPPAGE || slippage_ == 0) {
revert INVALID_GLOBAL_SLIPPAGE();
}

GLOBAL_SLIPPAGE = slippage_;
}

//////////////////////////////////////////////////////////////
// INTERNAL FUNCTIONS //
//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -439,6 +457,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
if (req.superformData.liqRequests[i].liqDstChainId != CHAIN_ID) {
revert REBALANCE_MULTI_POSITIONS_DIFFERENT_CHAIN();
}

if (req.superformData.amounts[i] != args.sharesToRedeem[i]) {
revert REBALANCE_MULTI_POSITIONS_DIFFERENT_AMOUNTS();
}
Expand All @@ -450,23 +469,29 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
/// @dev send SPs to router
_callSuperformRouter(router_, callData, args.rebalanceFromMsgValue);

uint256 amountToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;
uint256 availableBalanceToDeposit = interimAsset.balanceOf(address(this)) - args.balanceBefore;

if (amountToDeposit == 0) revert Error.ZERO_AMOUNT();
if (availableBalanceToDeposit == 0) revert Error.ZERO_AMOUNT();

if (
ENTIRE_SLIPPAGE * amountToDeposit
ENTIRE_SLIPPAGE * availableBalanceToDeposit
< ((args.expectedAmountToReceivePostRebalanceFrom * (ENTIRE_SLIPPAGE - args.slippage)))
) {
revert Error.VAULT_IMPLEMENTATION_FAILED();
}

/// @dev step 3: rebalance into a new superform with rebalanceCallData
if (!whitelistedSelectors[Actions.DEPOSIT][_parseSelectorMem(rebalanceToCallData)]) {
revert INVALID_DEPOSIT_SELECTOR();
}
uint256 amountIn = _validateAndGetAmountIn(rebalanceToCallData, availableBalanceToDeposit);

_deposit(router_, interimAsset, amountIn, args.rebalanceToMsgValue, rebalanceToCallData);
}

_deposit(router_, interimAsset, amountToDeposit, args.rebalanceToMsgValue, rebalanceToCallData);
function _takeAmountIn(LiqRequest memory liqReq, uint256 sfDataAmount) internal view returns (uint256 amountIn) {
bytes memory txData = liqReq.txData;
if (txData.length == 0) {
amountIn = sfDataAmount;
} else {
amountIn = IBridgeValidator(superRegistry.getBridgeValidator(liqReq.bridgeId)).decodeAmountIn(txData, false);
}
}

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

/// @dev validate the slippage
if (
(ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_))))
) {
if ((ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_))))) {
revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
}
}
Expand Down Expand Up @@ -604,7 +627,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {
/// @notice deposits ERC4626 vault shares into superform
/// @param vault_ The ERC4626 vault to redeem from
/// @param args Rest of the arguments to deposit 4626
function _deposit4626(address vault_, Deposit4626Args calldata args, uint256 arrayLength) internal {
function _deposit4626(address vault_, Deposit4626Args calldata args, uint256 arrayLength) internal {
_transferERC20In(IERC20(vault_), args.receiverAddressSP, args.amount);
IERC4626 vault = IERC4626(vault_);
address assetAdr = vault.asset();
Expand All @@ -614,12 +637,88 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus {

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

uint256 amountIn = _validateAndGetAmountIn(args.depositCallData, amountRedeemed);

uint256 msgValue = msg.value / arrayLength;
address router = _getAddress(keccak256("SUPERFORM_ROUTER"));
_deposit(router, asset, amountRedeemed, msgValue, args.depositCallData);

_deposit(router, asset, amountIn, msgValue, args.depositCallData);

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

emit Deposit4626Completed(args.receiverAddressSP, vault_);
}

function _validateAndGetAmountIn(
bytes calldata rebalanceToCallData,
uint256 availableBalanceToDeposit
)
internal
view
returns (uint256 amountIn)
{
bytes4 rebalanceToSelector = _parseSelectorMem(rebalanceToCallData);

if (!whitelistedSelectors[Actions.DEPOSIT][rebalanceToSelector]) {
revert INVALID_DEPOSIT_SELECTOR();
}

uint256 amountInTemp;

if (rebalanceToSelector == IBaseRouter.singleDirectSingleVaultDeposit.selector) {
SingleVaultSFData memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (SingleDirectSingleVaultStateReq)).superformData;
amountIn = _takeAmountIn(sfData.liqRequest, sfData.amount);
} else if (rebalanceToSelector == IBaseRouter.singleXChainSingleVaultDeposit.selector) {
SingleVaultSFData memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (SingleXChainSingleVaultStateReq)).superformData;
amountIn = _takeAmountIn(sfData.liqRequest, sfData.amount);
} else if (rebalanceToSelector == IBaseRouter.singleDirectMultiVaultDeposit.selector) {
MultiVaultSFData memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (SingleDirectMultiVaultStateReq)).superformData;
uint256 len = sfData.liqRequests.length;

for (uint256 i; i < len; ++i) {
amountInTemp = _takeAmountIn(sfData.liqRequests[i], sfData.amounts[i]);
amountIn += amountInTemp;
}
} else if (rebalanceToSelector == IBaseRouter.singleXChainMultiVaultDeposit.selector) {
MultiVaultSFData memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (SingleXChainMultiVaultStateReq)).superformsData;
uint256 len = sfData.liqRequests.length;
for (uint256 i; i < len; ++i) {
amountInTemp = _takeAmountIn(sfData.liqRequests[i], sfData.amounts[i]);
amountIn += amountInTemp;
}
} else if (rebalanceToSelector == IBaseRouter.multiDstSingleVaultDeposit.selector) {
SingleVaultSFData[] memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (MultiDstSingleVaultStateReq)).superformsData;
uint256 lenDst = sfData.length;
for (uint256 i; i < lenDst; ++i) {
amountInTemp = _takeAmountIn(sfData[i].liqRequest, sfData[i].amount);
amountIn += amountInTemp;
}
} else if (rebalanceToSelector == IBaseRouter.multiDstMultiVaultDeposit.selector) {
MultiVaultSFData[] memory sfData =
abi.decode(_parseCallData(rebalanceToCallData), (MultiDstMultiVaultStateReq)).superformsData;
uint256 lenDst = sfData.length;
for (uint256 i; i < lenDst; ++i) {
uint256 len = sfData[i].liqRequests.length;
for (uint256 j; j < len; ++j) {
amountInTemp = _takeAmountIn(sfData[i].liqRequests[j], sfData[i].amounts[j]);
amountIn += amountInTemp;
}
}
}

/// @dev amountIn must be artificially off-chain reduced to be less than availableBalanceToDeposit otherwise the
/// @dev approval to transfer tokens to SuperformRouter won't work
if (amountIn > availableBalanceToDeposit) revert AMOUNT_IN_NOT_EQUAL_OR_LOWER_THAN_BALANCE();

/// @dev check amountIn against availableBalanceToDeposit (available balance) via a GLOBAL_SLIPPAGE to prevent a
/// @dev malicious keeper from sending a low amountIn
if (ENTIRE_SLIPPAGE * amountIn < ((availableBalanceToDeposit * (ENTIRE_SLIPPAGE - GLOBAL_SLIPPAGE)))) {
revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE();
}
}
}
9 changes: 2 additions & 7 deletions src/router-plus/SuperformRouterPlusAsync.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
} from "src/router-plus/BaseSuperformRouterPlus.sol";
import { IBaseRouter } from "src/interfaces/IBaseRouter.sol";
import { ISuperformRouterPlusAsync } from "src/interfaces/ISuperformRouterPlusAsync.sol";
import { ISuperRBAC } from "src/interfaces/ISuperRBAC.sol";
import { SuperformFactory } from "src/SuperformFactory.sol";


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

/// @dev returns if an address has a specific role

function _hasRole(bytes32 id_, address addressToCheck_) internal view returns (bool) {
return ISuperRBAC(superRegistry.getAddress(keccak256("SUPER_RBAC"))).hasRole(id_, addressToCheck_);
}

function _castToMultiVaultData(SingleVaultSFData memory data_)
internal
pure
Expand Down
12 changes: 5 additions & 7 deletions test/fuzz/crosschain-data/adapters/HyperlaneImplementation.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,19 +107,17 @@ 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.stopPrank();

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.deal(malice_, 100 ether);
vm.prank(malice_);
Expand Down
2 changes: 0 additions & 2 deletions test/mainnet/SmokeTest.Staging.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,6 @@ contract SmokeTestStaging is MainnetBaseSetup {
}

function test_asyncStateRegistry() public {
AsyncStateRegistry asyncStateRegistry;

for (uint256 i; i < TARGET_DEPLOYMENT_CHAINS.length; ++i) {
uint64 chainId = TARGET_DEPLOYMENT_CHAINS[i];
vm.selectFork(FORKS[chainId]);
Expand Down
Loading