-
Notifications
You must be signed in to change notification settings - Fork 24
refactor: simplify test #50
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA single test file was restructured and updated to integrate TEE (Trusted Execution Environment) authenticated testing. Changes include reformatting imports and PDA derivations, adding TEE-related variable initialization, introducing a new test block for Player 2 airdropping, and systematically updating test flows to use programTee methods with accountsPartial for game creation, joining, making choices, and revealing winners. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-examples
🔇 Additional comments (6)
anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts (6)
1-51: LGTM!The imports, provider setup, and configuration are well-structured. The connection endpoint construction properly handles the trailing slash removal, and the ephemeral rollup provider is correctly instantiated with WebSocket endpoint configuration.
165-244: LGTM!The game creation flow is well-structured with proper permission setup, delegation, and error handling. The try/catch around
waitUntilPermissionActiveis a good defensive pattern.
246-337: LGTM!The Player 2 join flow mirrors the game creation pattern with consistent use of
accountsPartial, proper transaction submission, and appropriate error handling.
339-371: LGTM!The choice-making flow is clean and uses the TEE-authenticated programs correctly. The
accountsPartialpattern is consistent with other method calls.
401-428: LGTM!The sneak tests properly verify TEE privacy by confirming that players cannot read each other's choice accounts. The null check pattern and explicit error throwing are appropriate.
457-471: LGTM!The
Choicetype andgetRandomChoicehelper are correctly implemented following the Anchor enum pattern with empty object variants.
| // Permission TEE AuthToken | ||
| let authTokenPlayer1: { token: string; expiresAt: number }; | ||
| let authTokenPlayer2: { token: string; expiresAt: number }; | ||
| let programTeePlayer1: Program<AnchorRockPaperScissor>; | ||
| let programTeePlayer2: Program<AnchorRockPaperScissor>; | ||
|
|
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.
Potential undefined access for programTeePlayer1/programTeePlayer2.
These variables are only initialized when ephemeralRpcEndpoint.includes("tee") is true (lines 115-162), but they're used unconditionally in later tests (lines 341, 358, 374-376, 388-390, 403-405, 417-419, 431, 441-444). If running against a non-TEE endpoint, these tests will fail with cryptic undefined errors.
Consider initializing these to the regular program as a fallback or adding guards:
// Permission TEE AuthToken
let authTokenPlayer1: { token: string; expiresAt: number };
let authTokenPlayer2: { token: string; expiresAt: number };
- let programTeePlayer1: Program<AnchorRockPaperScissor>;
- let programTeePlayer2: Program<AnchorRockPaperScissor>;
+ let programTeePlayer1: Program<AnchorRockPaperScissor> = program;
+ let programTeePlayer2: Program<AnchorRockPaperScissor> = program;Alternatively, skip TEE-dependent tests when not using a TEE endpoint.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Permission TEE AuthToken | |
| let authTokenPlayer1: { token: string; expiresAt: number }; | |
| let authTokenPlayer2: { token: string; expiresAt: number }; | |
| let programTeePlayer1: Program<AnchorRockPaperScissor>; | |
| let programTeePlayer2: Program<AnchorRockPaperScissor>; | |
| // Permission TEE AuthToken | |
| let authTokenPlayer1: { token: string; expiresAt: number }; | |
| let authTokenPlayer2: { token: string; expiresAt: number }; | |
| let programTeePlayer1: Program<AnchorRockPaperScissor> = program; | |
| let programTeePlayer2: Program<AnchorRockPaperScissor> = program; |
🤖 Prompt for AI Agents
In anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts around lines 85
to 90, programTeePlayer1 and programTeePlayer2 are declared but only assigned
inside the ephemeralRpcEndpoint.includes("tee") branch, which will cause
undefined access later; fix by assigning a safe fallback immediately after
declaration (e.g., set programTeePlayer1 = program and programTeePlayer2 =
program) when not running against a TEE, or alternatively wrap/skip
TEE-dependent tests with a conditional that checks
ephemeralRpcEndpoint.includes("tee") so the tests never run against a non-TEE
endpoint.
| anchor.web3.SystemProgram.transfer({ | ||
| fromPubkey: player1.publicKey, | ||
| toPubkey: player2.publicKey, | ||
| lamports: 0.02 * anchor.web3.LAMPORTS_PER_SOL, // send 0.005 SOL |
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.
Comment/code mismatch.
The comment says "send 0.005 SOL" but the code transfers 0.02 SOL.
- lamports: 0.02 * anchor.web3.LAMPORTS_PER_SOL, // send 0.005 SOL
+ lamports: 0.02 * anchor.web3.LAMPORTS_PER_SOL, // send 0.02 SOL📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lamports: 0.02 * anchor.web3.LAMPORTS_PER_SOL, // send 0.005 SOL | |
| lamports: 0.02 * anchor.web3.LAMPORTS_PER_SOL, // send 0.02 SOL |
🤖 Prompt for AI Agents
In anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts around line 96,
the inline comment states "send 0.005 SOL" but the code uses 0.02 *
anchor.web3.LAMPORTS_PER_SOL; change the numeric value to 0.005 *
anchor.web3.LAMPORTS_PER_SOL so the code matches the comment (or alternatively
update the comment to "send 0.02 SOL" if the intended transfer is 0.02 SOL).
| programTeePlayer1 = new Program<AnchorRockPaperScissor>( | ||
| program.idl, | ||
| new anchor.AnchorProvider( | ||
| new anchor.web3.Connection( | ||
| process.env.EPHEMERAL_PROVIDER_ENDPOINT || | ||
| "https://tee.magicblock.app?token=" + authTokenPlayer1.token, | ||
| { | ||
| wsEndpoint: | ||
| process.env.EPHEMERAL_WS_ENDPOINT || | ||
| "wss://tee.magicblock.app?token=" + authTokenPlayer1.token, | ||
| } | ||
| ), | ||
| new anchor.Wallet(player1) | ||
| ), | ||
| program.coder | ||
| ); | ||
| programTeePlayer2 = new Program<AnchorRockPaperScissor>( | ||
| program.idl, | ||
| new anchor.AnchorProvider( | ||
| new anchor.web3.Connection( | ||
| process.env.EPHEMERAL_PROVIDER_ENDPOINT || | ||
| "https://tee.magicblock.app?token=" + authTokenPlayer2.token, | ||
| { | ||
| wsEndpoint: | ||
| process.env.EPHEMERAL_WS_ENDPOINT || | ||
| "wss://tee.magicblock.app?token=" + authTokenPlayer2.token, | ||
| } | ||
| ), | ||
| new anchor.Wallet(player2) | ||
| ), | ||
| program.coder | ||
| ); |
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.
Auth token not appended when EPHEMERAL_PROVIDER_ENDPOINT env var is set.
The || operator means that if process.env.EPHEMERAL_PROVIDER_ENDPOINT is defined, the token query parameter won't be appended, breaking TEE authentication. The same issue exists for both players and WebSocket endpoints.
Consider always appending the token to the configured endpoint:
programTeePlayer1 = new Program<AnchorRockPaperScissor>(
program.idl,
new anchor.AnchorProvider(
new anchor.web3.Connection(
- process.env.EPHEMERAL_PROVIDER_ENDPOINT ||
- "https://tee.magicblock.app?token=" + authTokenPlayer1.token,
+ (process.env.EPHEMERAL_PROVIDER_ENDPOINT || "https://tee.magicblock.app") +
+ "?token=" + authTokenPlayer1.token,
{
wsEndpoint:
- process.env.EPHEMERAL_WS_ENDPOINT ||
- "wss://tee.magicblock.app?token=" + authTokenPlayer1.token,
+ (process.env.EPHEMERAL_WS_ENDPOINT || "wss://tee.magicblock.app") +
+ "?token=" + authTokenPlayer1.token,
}
),
new anchor.Wallet(player1)
),
program.coder
);Apply the same fix for programTeePlayer2 on lines 146-161.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| programTeePlayer1 = new Program<AnchorRockPaperScissor>( | |
| program.idl, | |
| new anchor.AnchorProvider( | |
| new anchor.web3.Connection( | |
| process.env.EPHEMERAL_PROVIDER_ENDPOINT || | |
| "https://tee.magicblock.app?token=" + authTokenPlayer1.token, | |
| { | |
| wsEndpoint: | |
| process.env.EPHEMERAL_WS_ENDPOINT || | |
| "wss://tee.magicblock.app?token=" + authTokenPlayer1.token, | |
| } | |
| ), | |
| new anchor.Wallet(player1) | |
| ), | |
| program.coder | |
| ); | |
| programTeePlayer2 = new Program<AnchorRockPaperScissor>( | |
| program.idl, | |
| new anchor.AnchorProvider( | |
| new anchor.web3.Connection( | |
| process.env.EPHEMERAL_PROVIDER_ENDPOINT || | |
| "https://tee.magicblock.app?token=" + authTokenPlayer2.token, | |
| { | |
| wsEndpoint: | |
| process.env.EPHEMERAL_WS_ENDPOINT || | |
| "wss://tee.magicblock.app?token=" + authTokenPlayer2.token, | |
| } | |
| ), | |
| new anchor.Wallet(player2) | |
| ), | |
| program.coder | |
| ); | |
| programTeePlayer1 = new Program<AnchorRockPaperScissor>( | |
| program.idl, | |
| new anchor.AnchorProvider( | |
| new anchor.web3.Connection( | |
| (process.env.EPHEMERAL_PROVIDER_ENDPOINT || "https://tee.magicblock.app") + | |
| "?token=" + authTokenPlayer1.token, | |
| { | |
| wsEndpoint: | |
| (process.env.EPHEMERAL_WS_ENDPOINT || "wss://tee.magicblock.app") + | |
| "?token=" + authTokenPlayer1.token, | |
| } | |
| ), | |
| new anchor.Wallet(player1) | |
| ), | |
| program.coder | |
| ); | |
| programTeePlayer2 = new Program<AnchorRockPaperScissor>( | |
| program.idl, | |
| new anchor.AnchorProvider( | |
| new anchor.web3.Connection( | |
| (process.env.EPHEMERAL_PROVIDER_ENDPOINT || "https://tee.magicblock.app") + | |
| "?token=" + authTokenPlayer2.token, | |
| { | |
| wsEndpoint: | |
| (process.env.EPHEMERAL_WS_ENDPOINT || "wss://tee.magicblock.app") + | |
| "?token=" + authTokenPlayer2.token, | |
| } | |
| ), | |
| new anchor.Wallet(player2) | |
| ), | |
| program.coder | |
| ); |
🤖 Prompt for AI Agents
In anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts around lines 130
to 161, the current code uses `process.env.EPHEMERAL_PROVIDER_ENDPOINT ||
"https://tee.magicblock.app?token=" + authToken...` which prevents appending the
token when the env var is set; change to always append the token to the chosen
base endpoint for both player1 and player2 (and their wsEndpoint) by: pick the
base = process.env.EPHEMERAL_PROVIDER_ENDPOINT || "https://tee.magicblock.app",
then append the token query parameter safely (use the URL constructor or check
for existing query string to avoid duplicate '?' so result is base +
(base.includes('?') ? '&' : '?') + 'token=' + authTokenPlayerX.token); apply the
same pattern for EPHEMERAL_WS_ENDPOINT/wsEndpoint for both players.
| it("Player 1 checks own choice", async () => { | ||
| const accountInfo = await (authTokenPlayer1.token ? providerTeePlayer1 : provider).connection.getAccountInfo(player1ChoicePda); | ||
| const accountInfo = | ||
| await programTeePlayer1.provider.connection.getAccountInfo( | ||
| player1ChoicePda | ||
| ); | ||
| const player1ChoiceData = accountInfo.data; | ||
| const player1ChoiceAccount = program.account.playerChoice.coder.accounts.decode("playerChoice", player1ChoiceData); | ||
| const player1ChoiceAccount = | ||
| program.account.playerChoice.coder.accounts.decode( | ||
| "playerChoice", | ||
| player1ChoiceData | ||
| ); | ||
| console.log(`👀 Check Player 1 own Choice:`, player1ChoiceAccount.choice); | ||
| }); | ||
|
|
||
| it("Player 2 check own choice", async () => { | ||
| const accountInfo = await (authTokenPlayer2.token ? providerTeePlayer2 : provider).connection.getAccountInfo(player2ChoicePda); | ||
| it("Player 2 check own choice", async () => { | ||
| const accountInfo = | ||
| await programTeePlayer2.provider.connection.getAccountInfo( | ||
| player2ChoicePda | ||
| ); | ||
| const player2ChoiceData = accountInfo.data; | ||
| const player2ChoiceAccount = program.account.playerChoice.coder.accounts.decode("playerChoice", player2ChoiceData); | ||
| const player2ChoiceAccount = | ||
| program.account.playerChoice.coder.accounts.decode( | ||
| "playerChoice", | ||
| player2ChoiceData | ||
| ); | ||
| console.log(`👀 Check Player 2 own Choice:`, player2ChoiceAccount.choice); | ||
| }); |
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.
Missing null check on accountInfo before accessing .data.
getAccountInfo returns null if the account doesn't exist. Accessing accountInfo.data on lines 378 and 392 without a null check will cause a runtime error with an unclear message.
it("Player 1 checks own choice", async () => {
const accountInfo =
await programTeePlayer1.provider.connection.getAccountInfo(
player1ChoicePda
);
+ if (!accountInfo) {
+ throw new Error("Player 1 choice account not found");
+ }
const player1ChoiceData = accountInfo.data;Apply the same pattern for the Player 2 check on line 392.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts around lines 373
to 399, both Player 1 and Player 2 account retrievals call getAccountInfo and
immediately access accountInfo.data; add a null check after each getAccountInfo
call to handle the case where the account is missing (e.g., throw or assert with
a clear message including the PDA being fetched), and only proceed to decode and
log the account when accountInfo is non-null to avoid runtime errors.
| const accountInfo = await provider.connection.getAccountInfo(gamePda); | ||
| const gameData = accountInfo.data; | ||
| const gameAccount = program.account.game.coder.accounts.decode("game", gameData); | ||
| const gameAccount = program.account.game.coder.accounts.decode( | ||
| "game", | ||
| gameData | ||
| ); |
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.
Missing null check on accountInfo before accessing .data.
Similar to the choice checks, getAccountInfo can return null. Add a guard before accessing accountInfo.data.
const accountInfo = await provider.connection.getAccountInfo(gamePda);
+ if (!accountInfo) {
+ throw new Error("Game account not found after revealing winner");
+ }
const gameData = accountInfo.data;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const accountInfo = await provider.connection.getAccountInfo(gamePda); | |
| const gameData = accountInfo.data; | |
| const gameAccount = program.account.game.coder.accounts.decode("game", gameData); | |
| const gameAccount = program.account.game.coder.accounts.decode( | |
| "game", | |
| gameData | |
| ); | |
| const accountInfo = await provider.connection.getAccountInfo(gamePda); | |
| if (!accountInfo) { | |
| throw new Error("Game account not found after revealing winner"); | |
| } | |
| const gameData = accountInfo.data; | |
| const gameAccount = program.account.game.coder.accounts.decode( | |
| "game", | |
| gameData | |
| ); |
🤖 Prompt for AI Agents
In anchor-rock-paper-scissor/tests/anchor-rock-paper-scissor.ts around lines 447
to 452, accountInfo returned by provider.connection.getAccountInfo may be null
but the code accesses accountInfo.data directly; add a null check after the
await (throw or assert with a clear message if null) and only proceed to read
accountInfo.data and decode the game account when accountInfo is non-null to
avoid runtime exceptions.
This PR simplifies the rock paper scissor test
Summary by CodeRabbit
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.