Skip to content

Commit

Permalink
Merge pull request #642 from RigoBlock/fix/generic-fixes
Browse files Browse the repository at this point in the history 
fix: minor fixes
  • Loading branch information
@gabririgo
gabririgo authored Feb 17, 2025
2 parents da745e9 + a27377d commit 0c75b03
Show file tree
Hide file tree
Showing 14 changed files with 343 additions and 155 deletions.
26 changes: 10 additions & 16 deletions contracts/protocol/core/actions/MixinOwnerActions.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,40 +78,34 @@ abstract contract MixinOwnerActions is MixinActions {
}

// base token is never pushed to active list for gas savings, we can safely remove any unactive token
uint256 activeTokenLength = set.addresses.length;
address activeToken;
address[] memory activeTokens = set.addresses;
uint256 activeTokenBalance;

for (uint256 i = 0; i < activeTokenLength; i++) {
activeToken = set.addresses[i];
bool shouldRemove = true;
for (uint256 i = 0; i < activeTokens.length; i++) {
bool inApp;

// skip removal if a token is active in an application
for (uint256 j = 0; j < activeApps.length; j++) {
for (uint256 k = 0; k < activeApps[j].balances.length; k++) {
if (activeApps[j].balances[k].token == activeToken) {
shouldRemove = false;
if (activeApps[j].balances[k].token == activeTokens[i]) {
inApp = true;
break; // Exit k loop
}
}
if (!shouldRemove) {
if (inApp) {
break; // Exit j loop if token found in any app
}
}

if (shouldRemove) {
if (activeToken == _ZERO_ADDRESS) {
if (!inApp) {
if (activeTokens[i] == _ZERO_ADDRESS) {
activeTokenBalance = address(this).balance;
} else {
try IERC20(activeToken).balanceOf(address(this)) returns (uint256 _balance) {
activeTokenBalance = _balance;
} catch {
continue;
}
activeTokenBalance = IERC20(activeTokens[i]).balanceOf(address(this));
}

if (activeTokenBalance <= 1) {
set.remove(activeToken);
set.remove(activeTokens[i]);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/protocol/core/immutable/MixinImmutables.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pragma solidity >=0.8.0 <0.9.0;

import {MixinConstants} from "./MixinConstants.sol";
import {ISmartPoolImmutable} from "../../interfaces/pool/ISmartPoolImmutable.sol";
import {IExtensionsMap} from "../../extensions/IExtensionsMap.sol";
import {IExtensionsMap} from "../../interfaces/IExtensionsMap.sol";

/// @notice Immutables are not assigned a storage slot, can be safely added to this contract.
abstract contract MixinImmutables is MixinConstants {
Expand Down
12 changes: 5 additions & 7 deletions contracts/protocol/core/sys/MixinFallback.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import {VersionLib} from "../../libraries/VersionLib.sol";
abstract contract MixinFallback is MixinImmutables, MixinStorage {
using VersionLib for string;

error ExtensionsMapCallFailed();
error PoolImplementationDirectCallNotAllowed();
error PoolMethodNotAllowed();
error PoolVersionNotSupported();

// TODO: verify if this should be delegatecall restricted, as could be a repeated check in extensions/adapters
// reading immutable through internal method more gas efficient
modifier onlyDelegateCall() {
_checkDelegateCall();
Expand All @@ -28,9 +28,7 @@ abstract contract MixinFallback is MixinImmutables, MixinStorage {
/// @dev uses shouldDelegatecall to flag selectors that should prompt a delegatecall.
fallback() external payable onlyDelegateCall {
// returns nil target if selector not mapped. Uses delegatecall to preserve context of msg.sender for shouldDelegatecall flag
(bool success, bytes memory returnData) = address(_extensionsMap).delegatecall(abi.encodeCall(_extensionsMap.getExtensionBySelector, (msg.sig)));
// TODO: we probably do now need to assert success, as ExtensionsMap is hardcoded
require(success, ExtensionsMapCallFailed());
(, bytes memory returnData) = address(_extensionsMap).delegatecall(abi.encodeCall(_extensionsMap.getExtensionBySelector, (msg.sig)));
(address target, bool shouldDelegatecall) = abi.decode(returnData, (address, bool));

if (target == _ZERO_ADDRESS) {
Expand All @@ -44,13 +42,13 @@ abstract contract MixinFallback is MixinImmutables, MixinStorage {
require(VERSION.isVersionHigherOrEqual(required), PoolVersionNotSupported());
} catch {}

// adapter calls are aimed at pool operator use and for offchain inspection
shouldDelegatecall = pool().owner == msg.sender;
// adapter calls are for owner in write mode, and read mode for everyone else (including this)
shouldDelegatecall = msg.sender == pool().owner;
}

assembly {
calldatacopy(0, 0, calldatasize())
//let success
let success
if eq(shouldDelegatecall, 1) {
success := delegatecall(gas(), target, 0, calldatasize(), 0, 0)
returndatacopy(0, 0, returndatasize())
Expand Down
2 changes: 1 addition & 1 deletion contracts/protocol/extensions/ExtensionsMap.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
pragma solidity 0.8.28;

import {IAuthority} from "../interfaces/IAuthority.sol";
import {IExtensionsMap} from "../interfaces/IExtensionsMap.sol";
import {StorageLib} from "../libraries/StorageLib.sol";
import {IEApps} from "./adapters/interfaces/IEApps.sol";
import {IEOracle} from "./adapters/interfaces/IEOracle.sol";
import {IEUpgrade} from "./adapters/interfaces/IEUpgrade.sol";
import "./IExtensionsMap.sol";

/// @title ExtensionsMap - Wraps extensions selectors to addresses.
/// @author Gabriele Rigo - <gab@rigoblock.com>
Expand Down
104 changes: 60 additions & 44 deletions contracts/protocol/extensions/adapters/AUniswapDecoder.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ abstract contract AUniswapDecoder {
error OnlyOneMintOrBurnPerAction();

address internal constant ZERO_ADDRESS = address(0);
address internal constant MINT_AND_INCREASE_FLAG = address(1);
address private immutable _wrappedNative;

constructor(address wrappedNative) {
Expand Down Expand Up @@ -193,48 +194,43 @@ abstract contract AUniswapDecoder {
bytes calldata paramsAtIndex = encodedParams[actionIndex];

if (action < Actions.SETTLE) {
// no further assertion needed
//if (action == Actions.SWAP_EXACT_IN) {
//} else if (action == Actions.SWAP_EXACT_IN_SINGLE) {
//} else if (action == Actions.SWAP_EXACT_OUT) {
//} else if (action == Actions.SWAP_EXACT_OUT_SINGLE) {
//}
continue;
// no further decoding is needed, as we retrieve net values in payments actions
if (action == Actions.SWAP_EXACT_IN) {
continue;
} else if (action == Actions.SWAP_EXACT_IN_SINGLE) {
continue;
} else if (action == Actions.SWAP_EXACT_OUT) {
continue;
} else if (action == Actions.SWAP_EXACT_OUT_SINGLE) {
continue;
}
} else {
if (action == Actions.SETTLE_PAIR) {
revert UnsupportedAction(action);
} else if (action == Actions.TAKE_PAIR) {
revert UnsupportedAction(action);
} else if (action == Actions.SETTLE) {
// Currency currency, uint256 amount, bool payerIsUser
(Currency currency, uint256 amount,) =
abi.decode(paramsAtIndex, (Currency, uint256, bool));
if (action == Actions.SETTLE_ALL) {
(Currency currency, uint256 maxAmount) = paramsAtIndex.decodeCurrencyAndUint256();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency));
params.value += Currency.unwrap(currency) == ZERO_ADDRESS ? amount : 0;
params.value += Currency.unwrap(currency) == ZERO_ADDRESS ? maxAmount : 0;
continue;
} else if (action == Actions.TAKE) {
// Currency currency, address recipient, uint256 amount
(Currency currency, address recipient,) =
abi.decode(paramsAtIndex, (Currency, address, uint256));
} else if (action == Actions.TAKE_ALL) {
(Currency currency,) = paramsAtIndex.decodeCurrencyAndUint256();
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
params.recipients = _addUnique(params.recipients, recipient);
continue;
} else if (action == Actions.CLOSE_CURRENCY) {
// TODO: this won't be able to settle native, so need to make sure we forward ETH, i.e. eth must be added for single swaps
// this will either settle or take, so we need to make sure the token is tracked
(Currency currency) = paramsAtIndex.decodeCurrency();
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
} else if (action == Actions.SETTLE) {
(Currency currency, uint256 amount, bool payerIsUser) = paramsAtIndex.decodeCurrencyUint256AndBool();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency));
params.value += Currency.unwrap(currency) == ZERO_ADDRESS ? (payerIsUser ? amount : 0) : 0;
continue;
} else if (action == Actions.CLEAR_OR_TAKE) {
// Currency currency, uint256 amountMax
(Currency currency,) = paramsAtIndex.decodeCurrencyAndUint256();
} else if (action == Actions.TAKE) {
(Currency currency, address recipient,) = paramsAtIndex.decodeCurrencyAddressAndUint256();
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
params.recipients = _addUnique(params.recipients, recipient);
continue;
} else if (action == Actions.SWEEP) {
(Currency currency, address to) = paramsAtIndex.decodeCurrencyAndAddress();
} else if (action == Actions.TAKE_PORTION) {
(Currency currency, address recipient,) = paramsAtIndex.decodeCurrencyAddressAndUint256();
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
params.recipients = _addUnique(params.recipients, to);
params.recipients = _addUnique(params.recipients, recipient);
continue;
} else {
revert UnsupportedAction(action);
}
}
}
Expand Down Expand Up @@ -273,6 +269,8 @@ abstract contract AUniswapDecoder {
uint256 action;
}

/// @dev It cannot handle minting and increasing liquidity in the same call, as we run decoding before forwarding the call, hence the position is not
/// stored, and cannot return pool and position info, which are necessary to append value
function _decodePosmAction(
uint256 action,
bytes calldata actionParams,
Expand All @@ -282,42 +280,55 @@ abstract contract AUniswapDecoder {
if (action < Actions.SETTLE) {
if (action == Actions.INCREASE_LIQUIDITY) {
// uint256 tokenId, uint256 liquidity, uint128 amount0Max, uint128 amount1Max, bytes calldata hookData
(uint256 tokenId,,,,) = actionParams.decodeModifyLiquidityParams();
positions = _addUniquePosition(positions, Position(ZERO_ADDRESS, tokenId, Actions.INCREASE_LIQUIDITY));
(uint256 tokenId,, uint128 amount0Max,,) = actionParams.decodeModifyLiquidityParams();
(PoolKey memory poolKey,) = uniV4Posm().getPoolAndPositionInfo(tokenId);
params.value += Currency.unwrap(poolKey.currency0) == ZERO_ADDRESS ? amount0Max : 0;
// address(1) is used as a flag for when trying to increase liquidity on a non-minted pool
positions = _addUniquePosition(
positions,
Position(
Currency.unwrap(poolKey.currency1) != ZERO_ADDRESS ? address(poolKey.hooks) : MINT_AND_INCREASE_FLAG,
tokenId,
Actions.INCREASE_LIQUIDITY
)
);
return (params, positions);
} else if (action == Actions.INCREASE_LIQUIDITY_FROM_DELTAS) {
revert UnsupportedAction(action);
} else if (action == Actions.DECREASE_LIQUIDITY) {
// uint256 tokenId, uint256 liquidity, uint128 amount0Min, uint128 amount1Min, bytes calldata hookData
(uint256 tokenId,,,,) = actionParams.decodeModifyLiquidityParams();
// hook address is not relevant for decrease, use ZERO_ADDRESS instead to save gas
positions = _addUniquePosition(positions, Position(ZERO_ADDRESS, tokenId, Actions.DECREASE_LIQUIDITY));
return (params, positions);
} else if (action == Actions.MINT_POSITION) {
// PoolKey calldata poolKey, int24 tickLower, int24 tickUpper, uint256 liquidity, uint128 amount0Max, uint128 amount1Max, address owner, bytes calldata hookData
(PoolKey calldata poolKey,,,,,, address owner,) = actionParams.decodeMintParams();
(PoolKey calldata poolKey,,,, uint128 amount0Max,, address owner,) = actionParams.decodeMintParams();

// as an amount could be null, we want to assert here that both tokens have a price feed
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(poolKey.currency0));
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(poolKey.currency1));
params.recipients = _addUnique(params.recipients, owner);
params.value += Currency.unwrap(poolKey.currency0) == ZERO_ADDRESS ? amount0Max : 0;
// can only mint 1 liquidity position per modifyLiquidities transaction
positions = _addUniquePosition(positions, Position(address(poolKey.hooks), uniV4Posm().nextTokenId(), Actions.MINT_POSITION));
return (params, positions);
} else if (action == Actions.MINT_POSITION_FROM_DELTAS) {
revert UnsupportedAction(action);
} else if (action == Actions.BURN_POSITION) {
// uint256 tokenId, uint128 amount0Min, uint128 amount1Min, bytes calldata hookData
(uint256 tokenId,,,) = actionParams.decodeBurnParams();
// hook address is not relevant for burn, use ZERO_ADDRESS instead to save gas
positions = _addUniquePosition(positions, Position(ZERO_ADDRESS, tokenId, Actions.BURN_POSITION));
return (params, positions);
} else {
revert UnsupportedAction(action);
}
} else {
// TODO: verify if should revert following 2 methods, as prob used for migrations only?
if (action == Actions.SETTLE_PAIR) {
// settlement eth value must be retrieved in previous actions
(Currency currency0, Currency currency1) = actionParams.decodeCurrencyPair();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency0));
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency1));
// TODO: how do we get value for pair here?
//params.value += Currency.unwrap(currency0) == ZERO_ADDRESS ? amount : 0;
return (params, positions);
} else if (action == Actions.TAKE_PAIR) {
(Currency currency0, Currency currency1, address recipient) = actionParams.decodeCurrencyPairAndAddress();
Expand All @@ -326,20 +337,23 @@ abstract contract AUniswapDecoder {
params.recipients = _addUnique(params.recipients, recipient);
return (params, positions);
} else if (action == Actions.SETTLE) {
(Currency currency, uint256 amount,) = actionParams.decodeCurrencyUint256AndBool();
// in posm, SETTLE is usually used with ActionConstants.OPEN_DELTA (i.e. 0)
(Currency currency, uint256 amount, bool payerIsUser) = actionParams.decodeCurrencyUint256AndBool();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency));
params.value += Currency.unwrap(currency) == ZERO_ADDRESS ? amount : 0;
params.value += Currency.unwrap(currency) == ZERO_ADDRESS ? (payerIsUser ? amount : 0) : 0;
return (params, positions);
} else if (action == Actions.TAKE) {
(Currency currency, address recipient, /*uint256 amount*/) = actionParams.decodeCurrencyAddressAndUint256();
(Currency currency, address recipient,) = actionParams.decodeCurrencyAddressAndUint256();
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
params.recipients = _addUnique(params.recipients, recipient);
return (params, positions);
} else if (action == Actions.CLOSE_CURRENCY) {
// TODO: verify
revert UnsupportedAction(action);
Currency currency = actionParams.decodeCurrency();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency));
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
} else if (action == Actions.CLEAR_OR_TAKE) {
(Currency currency,) = actionParams.decodeCurrencyAndUint256();
params.tokensIn = _addUnique(params.tokensIn, Currency.unwrap(currency));
params.tokensOut = _addUnique(params.tokensOut, Currency.unwrap(currency));
return (params, positions);
} else if (action == Actions.SWEEP) {
Expand All @@ -355,6 +369,8 @@ abstract contract AUniswapDecoder {
} else if (action == Actions.UNWRAP) {
params.tokensOut = _addUnique(params.tokensOut, ZERO_ADDRESS);
return (params, positions);
} else {
revert UnsupportedAction(action);
}
}
revert UnsupportedAction(action);
Expand Down
Loading

0 comments on commit 0c75b03

Please sign in to comment.