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

position descriptor proxy tests #427

Merged
merged 14 commits into from
Jan 17, 2025
Merged
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly
version: stable

- name: Check format
run: forge fmt --check
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: nightly
version: stable

- name: Run tests
run: forge test --isolate -vvv
Expand Down
4 changes: 2 additions & 2 deletions script/DeployPosm.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ contract DeployPosmTest is Script {
address permit2,
uint256 unsubscribeGasLimit,
address wrappedNative,
string memory nativeCurrencyLabel
bytes32 nativeCurrencyLabelBytes
) public returns (IPositionDescriptor positionDescriptor, IPositionManager posm) {
vm.startBroadcast();

positionDescriptor = Deploy.positionDescriptor(poolManager, wrappedNative, nativeCurrencyLabel, hex"00");
positionDescriptor = Deploy.positionDescriptor(poolManager, wrappedNative, nativeCurrencyLabelBytes, hex"00");
console2.log("PositionDescriptor", address(positionDescriptor));

posm = Deploy.positionManager(
Expand Down
4 changes: 2 additions & 2 deletions snapshots/PosMGasTest.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
"PositionManager_mint_withClose": "421910",
"PositionManager_mint_withSettlePair": "420939",
"PositionManager_multicall_initialize_mint": "458237",
"PositionManager_permit": "79175",
"PositionManager_permit_secondPosition": "62063",
"PositionManager_permit": "79163",
"PositionManager_permit_secondPosition": "62075",
"PositionManager_permit_twice": "44975",
"PositionManager_subscribe": "87968",
"PositionManager_unsubscribe": "62697",
Expand Down
5 changes: 3 additions & 2 deletions snapshots/PositionDescriptorTest.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"position descriptor initcode hash (without constructor params, as uint256)": "88482580191959945449130293370700011665153263709859488839371600440410373093991",
"positionDescriptor bytecode size": "24110"
"position descriptor initcode hash (without constructor params, as uint256)": "62242073579002066175147720437850721791653757771535856244007756965162378120375",
"positionDescriptor bytecode size": "24069",
"proxy bytecode size": "1488"
}
23 changes: 18 additions & 5 deletions src/PositionDescriptor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,27 @@ contract PositionDescriptor is IPositionDescriptor {
address private constant WBTC = 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599;

address public immutable wrappedNative;
string public nativeCurrencyLabel;
bytes32 private immutable nativeCurrencyLabelBytes;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this private?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can make public?

Copy link
Contributor Author

@dianakocsis dianakocsis Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

daniel suggested we make it private since we already have a public getter for nativeCurrencyLabel

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont see why it can't just be public tho.. theres no downside


IPoolManager public immutable poolManager;

constructor(IPoolManager _poolManager, address _wrappedNative, string memory _nativeCurrencyLabel) {
constructor(IPoolManager _poolManager, address _wrappedNative, bytes32 _nativeCurrencyLabelBytes) {
poolManager = _poolManager;
wrappedNative = _wrappedNative;
nativeCurrencyLabel = _nativeCurrencyLabel;
nativeCurrencyLabelBytes = _nativeCurrencyLabelBytes;
}

/// @notice Returns the native currency label as a string
function nativeCurrencyLabel() public view returns (string memory) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would love to see unit tests for this function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint256 len = 0;
while (len < 32 && nativeCurrencyLabelBytes[len] != 0) {
len++;
}
bytes memory b = new bytes(len);
for (uint256 i = 0; i < len; i++) {
b[i] = nativeCurrencyLabelBytes[i];
}
return string(b);
}

/// @inheritdoc IPositionDescriptor
Expand Down Expand Up @@ -66,8 +79,8 @@ contract PositionDescriptor is IPositionDescriptor {
tokenId: tokenId,
quoteCurrency: quoteCurrency,
baseCurrency: baseCurrency,
quoteCurrencySymbol: SafeCurrencyMetadata.currencySymbol(quoteCurrency, nativeCurrencyLabel),
baseCurrencySymbol: SafeCurrencyMetadata.currencySymbol(baseCurrency, nativeCurrencyLabel),
quoteCurrencySymbol: SafeCurrencyMetadata.currencySymbol(quoteCurrency, nativeCurrencyLabel()),
baseCurrencySymbol: SafeCurrencyMetadata.currencySymbol(baseCurrency, nativeCurrencyLabel()),
quoteCurrencyDecimals: SafeCurrencyMetadata.currencyDecimals(quoteCurrency),
baseCurrencyDecimals: SafeCurrencyMetadata.currencyDecimals(baseCurrency),
flipRatio: _flipRatio,
Expand Down
74 changes: 44 additions & 30 deletions test/PositionDescriptor.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import {Descriptor} from "../src/libraries/Descriptor.sol";
contract PositionDescriptorTest is Test, PosmTestSetup {
using Base64 for string;

address public WETH9 = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
address public WETH9 = makeAddr("WETH");
address public DAI = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
address public USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
address public USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7;
address public TBTC = 0x8dAEBADE922dF735c38C80C7eBD708Af50815fAa;
address public WBTC = 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599;
string public nativeCurrencyLabel = "ETH";
bytes32 public nativeCurrencyLabelBytes = "ETH";

struct Token {
string description;
Expand All @@ -51,43 +52,56 @@ contract PositionDescriptorTest is Test, PosmTestSetup {
vm.snapshotValue("positionDescriptor bytecode size", address(positionDescriptor).code.length);
}

function test_bytecodeSize_proxy() public {
vm.snapshotValue("proxy bytecode size", address(proxyAsImplementation).code.length);
}

function test_setup_succeeds() public view {
assertEq(address(positionDescriptor.poolManager()), address(manager));
assertEq(positionDescriptor.wrappedNative(), WETH9);
assertEq(positionDescriptor.nativeCurrencyLabel(), nativeCurrencyLabel);
assertEq(address(proxyAsImplementation.poolManager()), address(manager));
assertEq(proxyAsImplementation.wrappedNative(), WETH9);
}

function test_nativeCurrencyLabel_succeeds() public {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added the unit test here!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update test to also test the raw bytes32

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cant since its private

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless we change to public

assertEq(proxyAsImplementation.nativeCurrencyLabel(), nativeCurrencyLabel);
IPositionDescriptor polDescriptor = deployDescriptor(manager, "POL");
assertEq(polDescriptor.nativeCurrencyLabel(), "POL");
IPositionDescriptor bnbDescriptor = deployDescriptor(manager, "BNB");
assertEq(bnbDescriptor.nativeCurrencyLabel(), "BNB");
IPositionDescriptor avaxDescriptor = deployDescriptor(manager, "AVAX");
assertEq(avaxDescriptor.nativeCurrencyLabel(), "AVAX");
}

function test_currencyRatioPriority_mainnet_succeeds() public {
vm.chainId(1);
assertEq(positionDescriptor.currencyRatioPriority(WETH9), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(positionDescriptor.currencyRatioPriority(address(0)), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(positionDescriptor.currencyRatioPriority(USDC), CurrencyRatioSortOrder.NUMERATOR_MOST);
assertEq(positionDescriptor.currencyRatioPriority(USDT), CurrencyRatioSortOrder.NUMERATOR_MORE);
assertEq(positionDescriptor.currencyRatioPriority(DAI), CurrencyRatioSortOrder.NUMERATOR);
assertEq(positionDescriptor.currencyRatioPriority(TBTC), CurrencyRatioSortOrder.DENOMINATOR_MORE);
assertEq(positionDescriptor.currencyRatioPriority(WBTC), CurrencyRatioSortOrder.DENOMINATOR_MOST);
assertEq(positionDescriptor.currencyRatioPriority(makeAddr("ALICE")), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(WETH9), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(proxyAsImplementation.currencyRatioPriority(address(0)), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(proxyAsImplementation.currencyRatioPriority(USDC), CurrencyRatioSortOrder.NUMERATOR_MOST);
assertEq(proxyAsImplementation.currencyRatioPriority(USDT), CurrencyRatioSortOrder.NUMERATOR_MORE);
assertEq(proxyAsImplementation.currencyRatioPriority(DAI), CurrencyRatioSortOrder.NUMERATOR);
assertEq(proxyAsImplementation.currencyRatioPriority(TBTC), CurrencyRatioSortOrder.DENOMINATOR_MORE);
assertEq(proxyAsImplementation.currencyRatioPriority(WBTC), CurrencyRatioSortOrder.DENOMINATOR_MOST);
assertEq(proxyAsImplementation.currencyRatioPriority(makeAddr("ALICE")), 0);
}

function test_currencyRatioPriority_notMainnet_succeeds() public {
assertEq(positionDescriptor.currencyRatioPriority(WETH9), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(positionDescriptor.currencyRatioPriority(address(0)), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(positionDescriptor.currencyRatioPriority(USDC), 0);
assertEq(positionDescriptor.currencyRatioPriority(USDT), 0);
assertEq(positionDescriptor.currencyRatioPriority(DAI), 0);
assertEq(positionDescriptor.currencyRatioPriority(TBTC), 0);
assertEq(positionDescriptor.currencyRatioPriority(WBTC), 0);
assertEq(positionDescriptor.currencyRatioPriority(makeAddr("ALICE")), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(WETH9), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(proxyAsImplementation.currencyRatioPriority(address(0)), CurrencyRatioSortOrder.DENOMINATOR);
assertEq(proxyAsImplementation.currencyRatioPriority(USDC), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(USDT), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(DAI), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(TBTC), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(WBTC), 0);
assertEq(proxyAsImplementation.currencyRatioPriority(makeAddr("ALICE")), 0);
}

function test_flipRatio_succeeds() public {
vm.chainId(1);
// bc price = token1/token0
assertTrue(positionDescriptor.flipRatio(USDC, WETH9));
assertFalse(positionDescriptor.flipRatio(DAI, USDC));
assertFalse(positionDescriptor.flipRatio(WBTC, WETH9));
assertFalse(positionDescriptor.flipRatio(WBTC, USDC));
assertFalse(positionDescriptor.flipRatio(WBTC, DAI));
assertTrue(proxyAsImplementation.flipRatio(USDC, WETH9));
assertFalse(proxyAsImplementation.flipRatio(DAI, USDC));
assertFalse(proxyAsImplementation.flipRatio(WBTC, WETH9));
assertFalse(proxyAsImplementation.flipRatio(WBTC, USDC));
assertFalse(proxyAsImplementation.flipRatio(WBTC, DAI));
}

function test_tokenURI_succeeds() public {
Expand All @@ -112,7 +126,7 @@ contract PositionDescriptorTest is Test, PosmTestSetup {
// The prefix length is calculated by converting the string to bytes and finding its length
uint256 prefixLength = bytes("data:application/json;base64,").length;

string memory uri = positionDescriptor.tokenURI(lpm, tokenId);
string memory uri = proxyAsImplementation.tokenURI(lpm, tokenId);
// Convert the uri to bytes
bytes memory uriBytes = bytes(uri);

Expand All @@ -133,7 +147,7 @@ contract PositionDescriptorTest is Test, PosmTestSetup {
}

// quote is currency1, base is currency0
assertFalse(positionDescriptor.flipRatio(Currency.unwrap(key.currency0), Currency.unwrap(key.currency1)));
assertFalse(proxyAsImplementation.flipRatio(Currency.unwrap(key.currency0), Currency.unwrap(key.currency1)));

string memory symbol0 = SafeCurrencyMetadata.currencySymbol(Currency.unwrap(currency0), nativeCurrencyLabel);
string memory symbol1 = SafeCurrencyMetadata.currencySymbol(Currency.unwrap(currency1), nativeCurrencyLabel);
Expand Down Expand Up @@ -231,7 +245,7 @@ contract PositionDescriptorTest is Test, PosmTestSetup {
// The prefix length is calculated by converting the string to bytes and finding its length
uint256 prefixLength = bytes("data:application/json;base64,").length;

string memory uri = positionDescriptor.tokenURI(lpm, tokenId);
string memory uri = proxyAsImplementation.tokenURI(lpm, tokenId);
// Convert the uri to bytes
bytes memory uriBytes = bytes(uri);

Expand All @@ -253,7 +267,7 @@ contract PositionDescriptorTest is Test, PosmTestSetup {

// quote is currency1, base is currency0
assertFalse(
positionDescriptor.flipRatio(Currency.unwrap(nativeKey.currency0), Currency.unwrap(nativeKey.currency1))
proxyAsImplementation.flipRatio(Currency.unwrap(nativeKey.currency0), Currency.unwrap(nativeKey.currency1))
);

string memory symbol0 =
Expand Down Expand Up @@ -354,7 +368,7 @@ contract PositionDescriptorTest is Test, PosmTestSetup {

vm.expectRevert(abi.encodeWithSelector(IPositionDescriptor.InvalidTokenId.selector, tokenId + 1));

positionDescriptor.tokenURI(lpm, tokenId + 1);
proxyAsImplementation.tokenURI(lpm, tokenId + 1);
}

// Helper functions for testing purposes
Expand Down
12 changes: 6 additions & 6 deletions test/position-managers/PositionManager.modifyLiquidities.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF

// Set the fee on transfer amount 1% higher.
(uint256 amount0, uint256 amount1) =
fotKey.currency0 == Currency.wrap(address(fotToken)) ? (100e18, 99e18) : (99e19, 100e18);
fotKey.currency0 == Currency.wrap(address(fotToken)) ? (100e18, 99e18) : (99e18, 100e18);

Plan memory planner = Planner.init();
planner.add(Actions.SETTLE, abi.encode(fotKey.currency0, amount0, true));
Expand All @@ -821,7 +821,7 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF
lpm.modifyLiquidities(actions, _deadline);

(uint256 amount0AfterTransfer, uint256 amount1AfterTransfer) =
fotKey.currency0 == Currency.wrap(address(fotToken)) ? (99e18, 100e18) : (100e18, 99e19);
fotKey.currency0 == Currency.wrap(address(fotToken)) ? (99e18, 100e18) : (100e18, 99e18);

uint128 newLiquidity = LiquidityAmounts.getLiquidityForAmounts(
SQRT_PRICE_1_1,
Expand Down Expand Up @@ -918,15 +918,15 @@ contract PositionManagerModifyLiquiditiesTest is Test, PosmTestSetup, LiquidityF
// Read below as:
// bool currency0IsFOT = fotKey.currency0 == Currency.wrap(address(fotToken));
// bool positionIsEntirelyInOtherToken = currency0IsFOT
// ? tickUpper <= TickMath.getTickAtSqrtPrice(sqrtPriceX96)
// : tickLower > TickMath.getTickAtSqrtPrice(sqrtPriceX96);
// ? TickMath.getSqrtPriceAtTick(tickUpper) <= sqrtPriceX96
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

// : TickMath.getSqrtPriceAtTick(tickLower) >= sqrtPriceX96;
// if (bips == 10000 && !positionIsEntirelyInOtherToken) {
if (
bips == 10000
&& !(
fotKey.currency0 == Currency.wrap(address(fotToken))
? tickUpper <= TickMath.getTickAtSqrtPrice(sqrtPriceX96)
: tickLower > TickMath.getTickAtSqrtPrice(sqrtPriceX96)
? TickMath.getSqrtPriceAtTick(tickUpper) <= sqrtPriceX96
: TickMath.getSqrtPriceAtTick(tickLower) >= sqrtPriceX96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES so much better thank you

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this i can sleep happy 😂

)
) {
vm.expectRevert(Position.CannotUpdateEmptyPosition.selector);
Expand Down
17 changes: 15 additions & 2 deletions test/shared/Deploy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {IPositionDescriptor} from "../../src/interfaces/IPositionDescriptor.sol"
import {IPositionManager} from "../../src/interfaces/IPositionManager.sol";
import {IV4Quoter} from "../../src/interfaces/IV4Quoter.sol";
import {IStateView} from "../../src/interfaces/IStateView.sol";
import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

library Deploy {
Vm internal constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));
Expand Down Expand Up @@ -41,13 +42,25 @@ library Deploy {
}
}

function transparentUpgradeableProxy(address implementation, address admin, bytes memory data, bytes memory salt)
internal
returns (TransparentUpgradeableProxy proxy)
{
bytes memory args = abi.encode(implementation, admin, data);
bytes memory initcode =
abi.encodePacked(vm.getCode("TransparentUpgradeableProxy.sol:TransparentUpgradeableProxy"), args);
assembly {
proxy := create2(0, add(initcode, 0x20), mload(initcode), salt)
}
}

function positionDescriptor(
address poolManager,
address wrappedNative,
string memory nativeCurrencyLabel,
bytes32 nativeCurrencyLabelBytes,
bytes memory salt
) internal returns (IPositionDescriptor descriptor) {
bytes memory args = abi.encode(poolManager, wrappedNative, nativeCurrencyLabel);
bytes memory args = abi.encode(poolManager, wrappedNative, nativeCurrencyLabelBytes);
bytes memory initcode = abi.encodePacked(vm.getCode("PositionDescriptor.sol:PositionDescriptor"), args);
assembly {
descriptor := create2(0, add(initcode, 0x20), mload(initcode), salt)
Expand Down
27 changes: 23 additions & 4 deletions test/shared/PosmTestSetup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,23 @@ import {MockERC20} from "solmate/src/test/utils/mocks/MockERC20.sol";
import {SortTokens} from "@uniswap/v4-core/test/utils/SortTokens.sol";
import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol";
import {PositionConfig} from "../shared/PositionConfig.sol";
import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

/// @notice A shared test contract that wraps the v4-core deployers contract and exposes basic liquidity operations on posm.
contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations {
uint256 constant STARTING_USER_BALANCE = 10_000_000 ether;

IAllowanceTransfer permit2;
IPositionDescriptor public positionDescriptor;
TransparentUpgradeableProxy proxy;
IPositionDescriptor proxyAsImplementation;
HookSavesDelta hook;
address hookAddr = address(uint160(Hooks.AFTER_ADD_LIQUIDITY_FLAG | Hooks.AFTER_REMOVE_LIQUIDITY_FLAG));
IWETH9 public _WETH9 = IWETH9(address(new WETH()));

WETH wethImpl = new WETH();
IWETH9 public _WETH9;

address governance = address(0xABCD);

HookModifyLiquidities hookModifyLiquidities;
address hookModifyLiquiditiesAddr = address(
Expand Down Expand Up @@ -67,13 +74,25 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations {
function deployPosm(IPoolManager poolManager) internal {
// We use deployPermit2() to prevent having to use via-ir in this repository.
permit2 = IAllowanceTransfer(deployPermit2());
positionDescriptor =
Deploy.positionDescriptor(address(poolManager), 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2, "ETH", hex"00");
_WETH9 = deployWETH();
proxyAsImplementation = deployDescriptor(poolManager, "ETH");
lpm = Deploy.positionManager(
address(poolManager), address(permit2), 100_000, address(positionDescriptor), address(_WETH9), hex"03"
address(poolManager), address(permit2), 100_000, address(proxyAsImplementation), address(_WETH9), hex"03"
);
}

function deployWETH() internal returns (IWETH9) {
address wethAddr = makeAddr("WETH");
vm.etch(wethAddr, address(wethImpl).code);
return IWETH9(wethAddr);
}

function deployDescriptor(IPoolManager poolManager, bytes32 label) internal returns (IPositionDescriptor) {
positionDescriptor = Deploy.positionDescriptor(address(poolManager), address(_WETH9), label, hex"00");
proxy = Deploy.transparentUpgradeableProxy(address(positionDescriptor), governance, "", hex"03");
return IPositionDescriptor(address(proxy));
}

function seedBalance(address to) internal {
IERC20(Currency.unwrap(currency0)).transfer(to, STARTING_USER_BALANCE);
IERC20(Currency.unwrap(currency1)).transfer(to, STARTING_USER_BALANCE);
Expand Down
Loading