-
Notifications
You must be signed in to change notification settings - Fork 509
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
Changes from 13 commits
8b1e603
b2c943b
6e67c90
ba6c3cd
6f05aaf
ccea1c1
4ddacf8
88617c1
a0268c2
76b0100
cce7374
bf6a92b
c109924
ce67772
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,14 +26,27 @@ contract PositionDescriptor is IPositionDescriptor { | |
address private constant WBTC = 0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599; | ||
|
||
address public immutable wrappedNative; | ||
string public nativeCurrencyLabel; | ||
bytes32 private immutable nativeCurrencyLabelBytes; | ||
|
||
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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would love to see unit tests for this function There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. code copied from here btw, but I couldn't find a unit test on the fly there either: |
||
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 | ||
|
@@ -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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i added the unit test here! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update test to also test the raw bytes32 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i cant since its private There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
@@ -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); | ||
|
||
|
@@ -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); | ||
|
@@ -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); | ||
|
||
|
@@ -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 = | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can make public?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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