Add unit tests for createPool.ts #501

calintje · 2024-11-13T14:58:47Z

Title
Add unit tests for createPool.ts

Details

Fixed the calculation of estInitializationCosts in the createConcentratedLiquidityPool function.
Added tests to verify the correct calculation of estInitialization costs.
Updated setupWhirlpool to support the V2 instruction of our program.

…ees.

wjthieme · 2024-11-13T15:00:33Z

ts-sdk/whirlpool/src/createPool.ts

-    .send();
+  const nonRefundableRents: Lamports[] = await Promise.all(
+    stateSpaces.map(async (space) => {
+      const rentExemption = await rpc.getMinimumBalanceForRentExemption(BigInt(space)).send();


I wouldn't do this since it would change the rpc calls it needs to make from 1 to 5 (or 6)

wjthieme · 2024-11-13T15:00:59Z

ts-sdk/whirlpool/src/createPool.ts

-  stateSpace += getWhirlpoolSize();
+  stateSpaces.push(
+    tokenProgramA === TOKEN_2022_PROGRAM_ADDRESS
+      ? getTokenSize(getAccountExtensions(mintA.data))


Can we make this a separate function (should be able to check mint.owner so mint would be the only parameter)?

Suspect we will need the same thing in increase_liquidity too. We can put it in token.ts

And then we can write a test for it 😄

wjthieme · 2024-11-13T15:02:34Z

ts-sdk/whirlpool/tests/createPool.test.ts

+    setDefaultFunder(signer);
+  });
+
+  it("Should throw an error if token mints are not ordered", async () => {


nit: they are ordered, just not ordered correctly

wjthieme · 2024-11-13T15:03:23Z

ts-sdk/whirlpool/tests/createPool.test.ts

+    await assert.rejects(
+      createConcentratedLiquidityPoolInstructions(rpc, mintB, mintA, 64, 1),
+      {
+        name: 'AssertionError',


Not sure if it exists but is there a new AssertionError("Token order needs to be flipped to match the canonical ordering (i.e. sorted on the byte repr. of the mint pubkeys)")

wjthieme · 2024-11-13T15:03:50Z

ts-sdk/whirlpool/tests/createPool.test.ts

+      createConcentratedLiquidityPoolInstructions(rpc, mintB, mintA, 64, 1),
+      {
+        name: 'AssertionError',
+        message: 'Token order needs to be flipped to match the canonical ordering (i.e. sorted on the byte repr. of the mint pubkeys)'


nit: we use double quotes for strings in ts (but you can run yarn format and that should format your files correctly

wjthieme · 2024-11-13T15:07:29Z

ts-sdk/whirlpool/tests/createPool.test.ts

+      await createSplashPoolInstructions(rpc, mintA, mintB, price);
+    await sendTransaction(instructions);
+
+    const pool = await fetchWhirlpool(rpc, poolAddress);


I would structure the test like this:

create splash pool

fetch maybe pool before

fetch signer balance before

assert pool does not exist

send tx

fetch maybe pool

fetch signer blance after

assert pool exists

assert sqrt, mintA, mintB, and tick spacing

assert that estInitialization cost equals difference in signer balance

That way you don't need the Should estimate initialization costs correctly test case entirely

And then create more tests with similar structure for:

Splash pool with TE token

CL pool

CL pool with TE token

etc.

wjthieme · 2024-11-13T15:07:47Z

ts-sdk/whirlpool/tests/utils/program.ts

 import {
  SPLASH_POOL_TICK_SPACING,
  WHIRLPOOLS_CONFIG_ADDRESS,
 } from "../../src/config";
 import { tickIndexToSqrtPrice } from "@orca-so/whirlpools-core";
+import { fetchMint } from "@solana-program/token";
+import { a } from "vitest/dist/chunks/suite.B2jumIFP.js";


…xtensions array.

wjthieme · 2024-11-14T16:51:58Z

ts-sdk/whirlpool/src/createPool.ts

-    .send();
+  const nonRefundableRents: Lamports[] = await Promise.all(
+    stateSpaces.map(async (space) => {
+      const rentExemption = await calculateMinimumBalance(rpc, space);


This is still doing the same thing where it fetches the rent 5 (or 6 times).

The way I would do it. Change the function to:

export function calculateMinimumBalance( rent: Rent, dataSize: number, ): Lamports {

Fetcht he rent sysvar at the beginning of this function and then instead of keeping track of stateSpace: Array<bigint> you can just straight up keep track of estInitializationCost: Lamports.

And then instead of doing stateSpace.push(...) you can just do estInitializationCost += calculateMinimumBalance(rent, ...)

wjthieme · 2024-11-14T16:52:13Z

ts-sdk/whirlpool/src/sysvar.ts

+ * @param {number} dataSize - The size of the account data in bytes.
+ * @returns {Promise<BigInt>} The minimum balance required for rent exemption in lamports.
+ */
+export async function calculateMinimumBalance(


Can you write a test for this?

wjthieme · 2024-11-14T16:54:35Z

ts-sdk/whirlpool/tests/createPool.test.ts

+    const price = 10;
+    const sqrtPrice = priceToSqrtPrice(price, 6, 6);
+
+    let signerAccount = await rpc.getAccountInfo(signer.address).send();


Think nicer if we implement getBalance on the mockRpc so we don't have to duplicate this every time.

wjthieme · 2024-11-14T16:55:54Z

ts-sdk/whirlpool/tests/createPool.test.ts

+    assertAccountExists(pool);
+
+    signerAccount = await rpc.getAccountInfo(signer.address).send();
+    const balanceAfter = signerAccount.value?.lamports ?? lamports(0n);


nit: I would group all the fetch together before starting on any assertions. poolBefore together with balanceBefore, poolAfter together with balanceAfter.

wjthieme · 2024-11-14T16:56:42Z

ts-sdk/whirlpool/tests/createPool.test.ts

+    const balanceChange = balanceBefore - balanceAfter;
+    const txFee = 15000n; // 3 signing accounts * 5000 lamports
+    const minRentExempt = balanceChange - txFee;
+    assert.strictEqual(estInitializationCost, minRentExempt);


Let me see if I can make the tx fee go to 0 in bankrun so that this assertion can just become:

assert.strictEqual(estInitializationCost, balanceBefore - balanceAfter);

wjthieme · 2024-11-14T16:58:02Z

ts-sdk/whirlpool/tests/createPool.test.ts

+  });
+
+  it("Should create splash pool with 1 TE token", async () => {
+    const mint1 = await setupMint();


I would set up these mints in the beforeAll as well since that will get faster execution (init the TE mint once opposed to doing it for each TE test)

wjthieme · 2024-11-14T17:00:16Z

ts-sdk/whirlpool/tests/createPool.test.ts

+  });
+
+  it("Should create splash pool with 2 TE tokens", async () => {
+    const mint1 = await setupMintTEFee();


For the tests I would do (instead of trying to combine Transfer fee and 1/2 TE token tests):

1 TE token (without extensions)

2 TE tokens (without extensions)

1 TE token (with transfer fee)

wjthieme · 2024-11-14T17:02:21Z

ts-sdk/whirlpool/src/sysvar.ts

@@ -0,0 +1,28 @@
+import { fetchSysvarRent } from "@solana/sysvars";


nit: This should be re-exported by @solana/web3.js so you can use that as import

calintje added 7 commits November 10, 2024 14:12

Base createPool test

c202a75

Process feedback. Improve createPool.ts tests. Attempt modify sysvarF…

f777c27

…ees.

Fix calculation of initizialization costs. Add createPool tests.

a3eab20

Merge branch 'main' into calintje/ts-sdk-unit-tests

502aae4

Clean up

08186d7

Clean up

47eba59

Clean up

413c5b0

calintje requested review from yugure-orca, wjthieme and odcheung as code owners November 13, 2024 14:58

wjthieme reviewed Nov 13, 2024

View reviewed changes

calintje added 7 commits November 13, 2024 16:33

Resolve minor comments

de84176

Restructure tests. Add getTokenSizeByProgram function, including tests.

71a7d7b

Resolve comments. Fix minor bug tih token size calculation based on e…

51bc3a0

…xtensions array.

Format

e8902bc

Update doc on core-sdks (unrelated)

3b4d976

Update doc on core-sdks (unrelated)

7e18813

Format

9a32a10

wjthieme reviewed Nov 14, 2024

View reviewed changes

Resolve comments

d54d855

calintje requested review from philcchen and rawfalafel as code owners November 15, 2024 02:42

Merge branch 'main' into calintje/ts-sdk-unit-tests

4fa6b40

calintje marked this pull request as draft November 15, 2024 02:46

calintje added 3 commits November 15, 2024 03:59

Revert changes

5c1bfc0

Revert changes

ccffacd

Revert changes

55b81c6

calintje marked this pull request as ready for review November 15, 2024 03:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit tests for createPool.ts #501

Add unit tests for createPool.ts #501

calintje commented Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 13, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

wjthieme Nov 14, 2024

		@@ -0,0 +1,28 @@
		import { fetchSysvarRent } from "@solana/sysvars";

Add unit tests for createPool.ts #501

Are you sure you want to change the base?

Add unit tests for createPool.ts #501

Conversation

calintje commented Nov 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment