added ephemeral vrf example #46
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedFailed to post review comments WalkthroughThis pull request adds comprehensive delegated dice rolling functionality to a Solana program. Changes include a new React page component for delegated rolling, shared Solana utilities and configuration constants, updates to the existing dice roller page, program modifications with new delegate parameters and account structures, expanded test coverage for delegation flows, and dependency version bumps for ephemeral rollups SDK support. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
roll-dice/README.md (1)
9-9: Remove the extraneous trailing slash in the URL.The link destination has a double trailing slash (
https://roll-dice-demo.vercel.app//), which is inconsistent with standard URL formatting. Update to a single trailing slash for consistency.Apply this diff to fix the URL:
-[https://roll-dice-demo.vercel.app/](https://roll-dice-demo.vercel.app//) +[https://roll-dice-demo.vercel.app/](https://roll-dice-demo.vercel.app/)roll-dice/app/package.json (1)
43-43: Avoid using"latest"for@solana/web3.js.Using
"latest"can cause unpredictable build failures when breaking changes are released. The Solana web3.js library recently underwent a major rewrite (v2.x), which has significant API differences from v1.x.Pin to a specific version to ensure reproducible builds:
- "@solana/web3.js": "latest", + "@solana/web3.js": "^1.98.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
roll-dice/Cargo.lockis excluded by!**/*.lockroll-dice/app/yarn.lockis excluded by!**/yarn.lock,!**/*.lockroll-dice/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
roll-dice/README.md(1 hunks)roll-dice/app/app/delegated/page.tsx(1 hunks)roll-dice/app/app/globals.css(1 hunks)roll-dice/app/app/page.tsx(4 hunks)roll-dice/app/lib/config.ts(1 hunks)roll-dice/app/lib/delegate-instruction.ts(1 hunks)roll-dice/app/lib/solana-utils.ts(1 hunks)roll-dice/app/lib/types.ts(1 hunks)roll-dice/app/package.json(1 hunks)roll-dice/package.json(1 hunks)roll-dice/programs/roll-dice-delegated/Cargo.toml(1 hunks)roll-dice/programs/roll-dice-delegated/src/lib.rs(6 hunks)roll-dice/tests/roll-dice-delegated.ts(3 hunks)roll-dice/vercel.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
roll-dice/app/app/delegated/page.tsx (8)
roll-dice/app/lib/config.ts (7)
PROGRAM_ID(3-3)ORACLE_QUEUE(6-6)BASE_ENDPOINT(7-7)PLAYER_STORAGE_KEY(8-8)BLOCKHASH_REFRESH_INTERVAL_MS(12-12)ROLL_ANIMATION_INTERVAL_MS(14-14)ROLL_TIMEOUT_MS(13-13)roll-dice/app/lib/types.ts (2)
RollEntry(1-7)CachedBlockhash(9-13)roll-dice/app/lib/solana-utils.ts (6)
getCachedBlockhash(55-70)checkDelegationStatus(72-78)walletAdapterFrom(6-18)fetchAndCacheBlockhash(39-53)loadOrCreateKeypair(20-29)ensureFunds(31-37)roll-dice/app/components/solana-address.tsx (1)
SolanaAddress(8-116)roll-dice/app/components/ui/card.tsx (3)
Card(79-79)CardTitle(79-79)CardContent(79-79)roll-dice/app/components/ui/accordion.tsx (4)
Accordion(58-58)AccordionItem(58-58)AccordionTrigger(58-58)AccordionContent(58-58)roll-dice/app/components/ui/badge.tsx (1)
Badge(36-36)roll-dice/app/components/ui/table.tsx (6)
Table(109-109)TableHeader(110-110)TableRow(114-114)TableHead(113-113)TableBody(111-111)TableCell(115-115)
roll-dice/app/lib/solana-utils.ts (2)
roll-dice/app/lib/config.ts (2)
MIN_BALANCE_LAMPORTS(10-10)BLOCKHASH_CACHE_MAX_AGE_MS(11-11)roll-dice/app/lib/types.ts (1)
CachedBlockhash(9-13)
roll-dice/app/app/page.tsx (2)
roll-dice/app/lib/solana-utils.ts (2)
loadOrCreateKeypair(20-29)walletAdapterFrom(6-18)roll-dice/app/lib/config.ts (4)
PLAYER_STORAGE_KEY(8-8)BASE_ENDPOINT(7-7)PROGRAM_ID_STANDARD(4-4)PLAYER_SEED(5-5)
roll-dice/tests/roll-dice-delegated.ts (1)
roll-dice/app/lib/delegate-instruction.ts (1)
createDelegateInstruction(37-105)
roll-dice/programs/roll-dice-delegated/src/lib.rs (1)
anchor-counter/programs/anchor-counter/src/lib.rs (2)
commit(50-58)delegate(36-47)
🪛 Biome (2.1.2)
roll-dice/app/app/delegated/page.tsx
[error] 873-876: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 873-876: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 888-892: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 896-900: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 903-907: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 910-914: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 918-922: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 954-958: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 968-980: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ 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 (14)
roll-dice/README.md (1)
13-13: Minor typo fix approved.The correction from "sofware" to "software" improves documentation clarity. Good catch!
roll-dice/vercel.json (1)
1-5: LGTM!The Vercel configuration correctly sets up the build command and output directory for the Next.js app located in the
appsubdirectory.roll-dice/app/lib/types.ts (1)
1-14: LGTM!The type definitions are clean and appropriate for their intended use cases—tracking roll history entries and caching blockhash data for transaction signing.
roll-dice/programs/roll-dice-delegated/Cargo.toml (1)
25-26: SDK versions are published and compatible.Both
ephemeral-rollups-sdk@0.4.1andephemeral-vrf-sdk@0.1.2are available on crates.io. The dependency updates align with the broader PR changes.roll-dice/app/lib/delegate-instruction.ts (1)
58-60: No action required — payer account writability is configured correctly.In Solana, payer accounts do not need to be marked
isWritable: truein instruction metadata. Runtime fees and rent deductions are handled automatically, and when a program uses CPI to create accounts (via SystemProgram), the inner instruction handles payer writability. The payer is correctly marked asisSigner: truefor authorization purposes.Likely an incorrect or invalid review comment.
roll-dice/tests/roll-dice-delegated.ts (1)
145-200: Disabled test for undelegating on-curve account.This test is marked with
xit(disabled). If this functionality is required, consider enabling and completing it. If it's intentionally disabled for now, add a comment explaining why.roll-dice/app/lib/config.ts (1)
1-14: LGTM overall structure.The centralized configuration module is well-organized with clear constant definitions for program IDs, seeds, endpoints, and timing values. This promotes maintainability and reduces magic values scattered across the codebase.
roll-dice/app/lib/solana-utils.ts (1)
31-37: Airdrop only works on devnet/testnet.
requestAirdropwill fail on mainnet. Consider adding a network check or documenting this limitation clearly. For production use, this function should be disabled or replaced with a different funding mechanism.export const ensureFunds = async (connection: Connection, keypair: Keypair): Promise<void> => { const balance = await connection.getBalance(keypair.publicKey) if (balance < MIN_BALANCE_LAMPORTS * LAMPORTS_PER_SOL) { + // Note: Airdrop only works on devnet/testnet, not mainnet const signature = await connection.requestAirdrop(keypair.publicKey, LAMPORTS_PER_SOL) await connection.confirmTransaction(signature, "confirmed") } }roll-dice/programs/roll-dice-delegated/src/lib.rs (6)
1-13: LGTM!Imports are appropriate for the VRF and ephemeral rollups functionality. The
SerializableAccountMetaimport supports the new account metadata in randomness requests.
37-41: LGTM!The
SerializableAccountMetacorrectly identifies the player account for the callback, marking it as writable (required since the callback modifies player state) and non-signer.
63-74: LGTM!The explicit
DelegateParamsstruct provides a cleaner API compared to usingremaining_accountsfor validator configuration. TheDelegateConfigis properly constructed from the provided parameters.
92-92: LGTM!Space calculation is correct: 8 bytes discriminator + 2 bytes for Player data (1 byte
last_result+ 1 byterollnum).
137-138: LGTM!The
#[account(mut)]constraint is correctly added since the callback modifies the player'srollnumandlast_resultfields.
166-170: LGTM!
DelegateParamscorrectly derivesAnchorSerializeandAnchorDeserializefor use as instruction arguments, providing a clean interface for delegation configuration.
| subscriptionIdRef.current = newEphemeralConnection.onAccountChange( | ||
| playerPdaRef.current, | ||
| (accountInfo) => { | ||
| if (!ephemeralProgramRef.current || !accountInfo?.data) return | ||
| try { | ||
| const player = ephemeralProgramRef.current.coder.accounts.decode("player", accountInfo.data) | ||
| const newValue = Number(player.lastResult) | ||
| setPlayerAccountData({ lastResult: newValue, rollnum: Number(player.rollnum) }) | ||
| if (newValue > 0) { | ||
| setDiceValue(newValue) | ||
| previousDiceValueRef.current = newValue | ||
| } | ||
| setRollHistory(prev => { | ||
| const idx = prev.findIndex(entry => entry.isPending) | ||
| if (idx === -1) return prev | ||
| const updated = [...prev] | ||
| updated[idx] = { ...updated[idx], value: newValue, endTime: Date.now(), isPending: false } | ||
| setIsRolling(false) | ||
| clearAllIntervals() | ||
| return updated | ||
| }) | ||
| } catch (error) { | ||
| console.error("[WebSocket] Failed to decode player account:", error) | ||
| } | ||
| }, | ||
| { commitment: "processed" } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicated WebSocket subscription handler logic.
The account change handler logic is duplicated between updateEphemeralConnectionToValidator (lines 249-275) and initializeProgram (lines 380-406). Extract this into a reusable helper function to improve maintainability.
const createAccountChangeHandler = () => (accountInfo: AccountInfo<Buffer>) => {
if (!ephemeralProgramRef.current || !accountInfo?.data) return
try {
const player = ephemeralProgramRef.current.coder.accounts.decode("player", accountInfo.data)
const newValue = Number(player.lastResult)
setPlayerAccountData({ lastResult: newValue, rollnum: Number(player.rollnum) })
if (newValue > 0) {
setDiceValue(newValue)
previousDiceValueRef.current = newValue
}
setRollHistory(prev => {
const idx = prev.findIndex(entry => entry.isPending)
if (idx === -1) return prev
const updated = [...prev]
updated[idx] = { ...updated[idx], value: newValue, endTime: Date.now(), isPending: false }
setIsRolling(false)
clearAllIntervals()
return updated
})
} catch (error) {
console.error("[WebSocket] Failed to decode player account:", error)
}
}Also applies to: 380-406
🤖 Prompt for AI Agents
In roll-dice/app/app/delegated/page.tsx around lines 249-275 (and similarly
380-406), the WebSocket account-change handler logic is duplicated; extract it
to a single reusable function (e.g., createAccountChangeHandler) that returns a
handler of type (accountInfo: AccountInfo<Buffer>) => void and closes over the
existing refs/state (ephemeralProgramRef, playerPdaRef, setPlayerAccountData,
setDiceValue, previousDiceValueRef, setRollHistory, setIsRolling,
clearAllIntervals), replace the duplicated blocks at both locations with calls
to newHandler = createAccountChangeHandler() and pass that handler to
onAccountChange with the same commitment option, and keep the try/catch and
console.error behavior intact so behavior does not change.
| } | ||
| setIsDelegating(false) | ||
| } | ||
| }, [isDelegated, refreshDelegationStatus, clearAllIntervals, updateEphemeralConnectionToValidator, sendBackgroundRoll]) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused dependency clearAllIntervals in callback.
clearAllIntervals is listed in the dependency array for handleDelegateToValidator but is never called within the function body. Remove it from deps or use it if intended.
- }, [isDelegated, refreshDelegationStatus, clearAllIntervals, updateEphemeralConnectionToValidator, sendBackgroundRoll])
+ }, [isDelegated, refreshDelegationStatus, updateEphemeralConnectionToValidator, sendBackgroundRoll])📝 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.
| }, [isDelegated, refreshDelegationStatus, clearAllIntervals, updateEphemeralConnectionToValidator, sendBackgroundRoll]) | |
| }, [isDelegated, refreshDelegationStatus, updateEphemeralConnectionToValidator, sendBackgroundRoll]) |
🤖 Prompt for AI Agents
In roll-dice/app/app/delegated/page.tsx around line 579, the dependency array
for the handleDelegateToValidator callback includes clearAllIntervals but that
function is never used inside the callback; remove clearAllIntervals from the
dependency array (or, if you intended to use it, call it inside the callback and
keep it in deps) and run the linter/tests to confirm no stale deps remain.
| // List of all known validator endpoints | ||
| const validatorEndpoints = [ | ||
| "https://devnet-us.magicblock.app", | ||
| "https://devnet-as.magicblock.app", | ||
| "https://devnet-eu.magicblock.app", | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hardcoded validator endpoints should use configuration.
The validator endpoints are hardcoded. Consider moving these to the config file alongside other endpoint constants for easier maintenance and environment-specific configuration.
+// In config.ts:
+export const VALIDATOR_ENDPOINTS = [
+ "https://devnet-us.magicblock.app",
+ "https://devnet-as.magicblock.app",
+ "https://devnet-eu.magicblock.app",
+]
// In page.tsx:
-const validatorEndpoints = [
- "https://devnet-us.magicblock.app",
- "https://devnet-as.magicblock.app",
- "https://devnet-eu.magicblock.app",
-]
+import { VALIDATOR_ENDPOINTS } from "@/lib/config"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In roll-dice/app/app/delegated/page.tsx around lines 639–644 the validator
endpoints are hardcoded; move them into configuration by adding a new constant
(e.g., VALIDATOR_ENDPOINTS) in the project config (or environment config) and
replace this inline array with an import from that config; support reading from
process.env (or NEXT_PUBLIC_ env) with a sensible default fallback so dev/prod
can differ, and update any type definitions/imports accordingly.
|
|
||
| try { | ||
| const randomValue = Math.floor(Math.random() * 6) + 1 | ||
| const connection = ephemeralConnectionRef.current! |
There was a problem hiding this comment.
Avoid non-null assertion; add guard clause.
ephemeralConnectionRef.current! uses a non-null assertion which could cause a runtime error if the ref is unexpectedly null. The guard at line 717 only checks ephemeralProgramRef, not ephemeralConnectionRef.
- const connection = ephemeralConnectionRef.current!
+ const connection = ephemeralConnectionRef.current
+ if (!connection) {
+ console.error("[RollDice] Ephemeral connection not available")
+ setIsRolling(false)
+ return
+ }🤖 Prompt for AI Agents
In roll-dice/app/app/delegated/page.tsx around line 764, avoid the non-null
assertion on ephemeralConnectionRef.current; add a guard check for
ephemeralConnectionRef (similar to the existing ephemeralProgramRef guard at
717) before using it. Replace the direct assignment with a guarded flow: if
ephemeralConnectionRef.current is null/undefined, handle it (early return, throw
a descriptive error, or log and exit) and only then assign const connection =
ephemeralConnectionRef.current without the !. Ensure the error message clearly
identifies that ephemeralConnectionRef is missing so callers can diagnose the
issue.
| <div | ||
| className="flex items-center gap-2 cursor-pointer hover:bg-gray-50 rounded px-2 py-1 transition-colors" | ||
| onClick={() => copyToClipboard(playerPda.toBase58())} | ||
| > | ||
| <span className="text-xs font-mono">{formatAddress(playerPda.toBase58())}</span> | ||
| {copied ? <Check className="h-3 w-3 text-green-500" /> : <Copy className="h-3 w-3 text-gray-400" />} | ||
| </div> |
There was a problem hiding this comment.
Add keyboard accessibility to clickable div.
The PDA address copy functionality uses a <div> with onClick but lacks keyboard support. Users navigating with keyboards cannot trigger this action. As flagged by static analysis.
<div
className="flex items-center gap-2 cursor-pointer hover:bg-gray-50 rounded px-2 py-1 transition-colors"
onClick={() => copyToClipboard(playerPda.toBase58())}
+ onKeyDown={(e) => e.key === 'Enter' && copyToClipboard(playerPda.toBase58())}
+ role="button"
+ tabIndex={0}
>📝 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.
| <div | |
| className="flex items-center gap-2 cursor-pointer hover:bg-gray-50 rounded px-2 py-1 transition-colors" | |
| onClick={() => copyToClipboard(playerPda.toBase58())} | |
| > | |
| <span className="text-xs font-mono">{formatAddress(playerPda.toBase58())}</span> | |
| {copied ? <Check className="h-3 w-3 text-green-500" /> : <Copy className="h-3 w-3 text-gray-400" />} | |
| </div> | |
| <div | |
| className="flex items-center gap-2 cursor-pointer hover:bg-gray-50 rounded px-2 py-1 transition-colors" | |
| onClick={() => copyToClipboard(playerPda.toBase58())} | |
| onKeyDown={(e) => e.key === 'Enter' && copyToClipboard(playerPda.toBase58())} | |
| role="button" | |
| tabIndex={0} | |
| > | |
| <span className="text-xs font-mono">{formatAddress(playerPda.toBase58())}</span> | |
| {copied ? <Check className="h-3 w-3 text-green-500" /> : <Copy className="h-3 w-3 text-gray-400" />} | |
| </div> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 873-876: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 873-876: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
🤖 Prompt for AI Agents
In roll-dice/app/app/delegated/page.tsx around lines 873 to 879, the clickable
PDA copy DIV only has an onClick handler and therefore lacks keyboard
accessibility; update the element to behave like a button by adding
role="button", tabIndex={0}, an aria-label (e.g., "Copy PDA address"), and an
onKeyDown handler that triggers the same copyToClipboard(playerPda.toBase58())
action when the Enter or Space keys are pressed; keep the existing onClick and
visual styling intact so both mouse and keyboard users can activate the control.
| "dependencies": { | ||
| "@coral-xyz/anchor": "0.32.1" | ||
| "@coral-xyz/anchor": "0.32.1", | ||
| "@magicblock-labs/ephemeral-rollups-sdk": "^0.4.1" | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM with minor note on version consistency.
The dependencies are appropriate for the ephemeral VRF integration. Consider using consistent version pinning strategy—@coral-xyz/anchor uses an exact version (0.32.1) while @magicblock-labs/ephemeral-rollups-sdk uses a caret range (^0.4.1).
🤖 Prompt for AI Agents
In roll-dice/package.json around lines 7 to 10, the dependency versioning is
inconsistent: "@coral-xyz/anchor" is pinned to "0.32.1" while
"@magicblock-labs/ephemeral-rollups-sdk" uses a caret "^0.4.1"; pick a
consistent versioning strategy and apply it to both dependencies (either pin
both exact versions or use caret ranges for both), updating the package.json
entries accordingly and run npm/yarn install to ensure lockfile consistency.
| pub fn callback_roll_dice_simple( | ||
| _ctx: Context<CallbackRollDiceSimpleCtx>, | ||
| ctx: Context<CallbackRollDiceSimpleCtx>, | ||
| randomness: [u8; 32], | ||
| ) -> Result<()> { | ||
| let rnd_u8 = ephemeral_vrf_sdk::rnd::random_u8_with_range(&randomness, 1, 6); | ||
| msg!("Consuming random number: {:?}", rnd_u8); | ||
| player.rollnum = player.rollnum.saturating_add(1); | ||
| msg!("Roll number: {:?}", player.rollnum); | ||
| let player = &mut ctx.accounts.player; | ||
| player.last_result = rnd_u8; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Critical: Variable used before declaration.
Lines 55-56 reference player before it is declared on line 57. This will fail to compile due to use of an undefined variable.
Apply this diff to fix the ordering:
pub fn callback_roll_dice_simple(
ctx: Context<CallbackRollDiceSimpleCtx>,
randomness: [u8; 32],
) -> Result<()> {
let rnd_u8 = ephemeral_vrf_sdk::rnd::random_u8_with_range(&randomness, 1, 6);
msg!("Consuming random number: {:?}", rnd_u8);
+ let player = &mut ctx.accounts.player;
player.rollnum = player.rollnum.saturating_add(1);
msg!("Roll number: {:?}", player.rollnum);
- let player = &mut ctx.accounts.player;
player.last_result = rnd_u8;
Ok(())
}📝 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.
| pub fn callback_roll_dice_simple( | |
| _ctx: Context<CallbackRollDiceSimpleCtx>, | |
| ctx: Context<CallbackRollDiceSimpleCtx>, | |
| randomness: [u8; 32], | |
| ) -> Result<()> { | |
| let rnd_u8 = ephemeral_vrf_sdk::rnd::random_u8_with_range(&randomness, 1, 6); | |
| msg!("Consuming random number: {:?}", rnd_u8); | |
| player.rollnum = player.rollnum.saturating_add(1); | |
| msg!("Roll number: {:?}", player.rollnum); | |
| let player = &mut ctx.accounts.player; | |
| player.last_result = rnd_u8; | |
| Ok(()) | |
| } | |
| pub fn callback_roll_dice_simple( | |
| ctx: Context<CallbackRollDiceSimpleCtx>, | |
| randomness: [u8; 32], | |
| ) -> Result<()> { | |
| let rnd_u8 = ephemeral_vrf_sdk::rnd::random_u8_with_range(&randomness, 1, 6); | |
| msg!("Consuming random number: {:?}", rnd_u8); | |
| let player = &mut ctx.accounts.player; | |
| player.rollnum = player.rollnum.saturating_add(1); | |
| msg!("Roll number: {:?}", player.rollnum); | |
| player.last_result = rnd_u8; | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In roll-dice/programs/roll-dice-delegated/src/lib.rs around lines 49-60, the
code references player before it is declared which causes a compile error; move
the declaration "let player = &mut ctx.accounts.player;" to before any use of
player (i.e., place it immediately after getting rnd_u8 and before the msg! and
player.rollnum modifications), then update subsequent accesses to use this
declared mutable reference so the borrow semantics remain correct.
| import { LAMPORTS_PER_SOL } from "@solana/web3.js"; | ||
| import { LAMPORTS_PER_SOL, Keypair, SystemProgram, Transaction, PublicKey } from "@solana/web3.js"; | ||
| import { RandomDiceDelegated } from "../target/types/random_dice_delegated"; | ||
| import * as crypto from "crypto"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import when test is disabled.
The crypto import is only used in the disabled xit test at line 145. Consider removing it or enabling the test.
🤖 Prompt for AI Agents
In roll-dice/tests/roll-dice-delegated.ts around line 5, the imported "crypto"
module is unused because the only test that references it is disabled (an xit at
~line 145); remove the unused import line to clean up linter/test warnings, or
alternatively re-enable the disabled test and ensure the crypto usage within
that test is correct — choose one: either delete the import statement at the
top, or change the test from xit to it and verify the test logic uses crypto as
intended.
| const delegateTxHash = await provider.sendAndConfirm(new Transaction().add(delegateIx), [provider.wallet.payer,delegatedPayerKeypair]); | ||
| console.log("Delegate transaction signature:", delegateTxHash); | ||
|
|
||
| await new Promise(resolve => setTimeout(resolve, 5000)); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting magic delay values.
The 5-second delays are hardcoded. Consider extracting these as constants or using a more robust approach like polling for expected state changes.
+const DELEGATION_PROPAGATION_DELAY_MS = 5000;
+
// In the test:
- await new Promise(resolve => setTimeout(resolve, 5000));
+ await new Promise(resolve => setTimeout(resolve, DELEGATION_PROPAGATION_DELAY_MS));Also applies to: 124-124
🤖 Prompt for AI Agents
In roll-dice/tests/roll-dice-delegated.ts around lines 103 and 124, there are
hardcoded 5000ms delays; extract these magic numbers into a named constant
(e.g., const DEFAULT_WAIT_MS = 5000) at the top of the test file or test
utilities, and replace the inline setTimeout usages with that constant, or
better yet implement a small helper that polls for the expected state (e.g.,
waitFor(conditionFn, {timeout: DEFAULT_WAIT_MS, interval: 100})) and use it to
wait for specific conditions instead of fixed sleeps.
| const undelegateAccounts = [ | ||
| { pubkey: providerEphemeralRollup.wallet.publicKey, isSigner: true, isWritable: true }, // payer | ||
| { pubkey: delegatedAccount, isSigner: false, isWritable: true }, // delegated_account | ||
| { pubkey: SystemProgram.programId, isSigner: false, isWritable: false }, // owner_program (system for on-curve) | ||
| { pubkey: delegationRecordPda, isSigner: false, isWritable: true }, // delegation_record | ||
| { pubkey: delegationMetadataPda, isSigner: false, isWritable: true }, // delegation_metadata | ||
| { pubkey: DELEGATION_PROGRAM_ID, isSigner: false, isWritable: false }, // delegation_program | ||
| { pubkey: MAGIC_PROGRAM_ID, isSigner: false, isWritable: false }, // magic_program | ||
| { pubkey: MAGIC_CONTEXT_ID, isSigner: false, isWritable: true }, // magic_context | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "undelegate" --type ts -C 3Repository: magicblock-labs/magicblock-engine-examples
Length of output: 22695
🏁 Script executed:
# Search for delegation program implementation and instruction definitions
rg -n "delegation" --type ts -i | head -50Repository: magicblock-labs/magicblock-engine-examples
Length of output: 5402
🏁 Script executed:
# Look for Anchor IDL or instruction definitions that might show signatures
fd -e json -e idl | xargs rg -l "undelegate" 2>/dev/nullRepository: magicblock-labs/magicblock-engine-examples
Length of output: 70
🏁 Script executed:
# Check if there's a delegation program directory or related files
fd -type f -name "*delegat*" | grep -E "\.(ts|rs|json|idl)$"Repository: magicblock-labs/magicblock-engine-examples
Length of output: 256
🏁 Script executed:
# Look for oncurve-delegation instruction creation to see account signing patterns
rg -n "createCommitAndUndelegateInstruction" --type ts -A 10 | head -60Repository: magicblock-labs/magicblock-engine-examples
Length of output: 4157
🏁 Script executed:
# Search for delegation program IDL or type definitions
fd -type f \( -name "*.json" -o -name "*.ts" \) | xargs rg -l "undelegate" | grep -E "(idl|types|interface)" | head -10Repository: magicblock-labs/magicblock-engine-examples
Length of output: 256
🏁 Script executed:
# Look for the delegation program source or accounts interface
fd -type f -name "*delegat*" -path "*/src/*"Repository: magicblock-labs/magicblock-engine-examples
Length of output: 256
🏁 Script executed:
# Check the specific line in roll-dice-delegated.ts to see the full context
sed -n '130,190p' roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 2780
🏁 Script executed:
# Search for createCommitAndUndelegateInstruction implementation
rg -n "createCommitAndUndelegateInstruction" --type ts -B 2 -A 15 | grep -A 15 "export\|function"Repository: magicblock-labs/magicblock-engine-examples
Length of output: 68
🏁 Script executed:
# Check if there's an SDK package in node_modules or imported from external package
rg "from.*ephemeral-rollups" roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 134
🏁 Script executed:
# Look for any comments or documentation about undelegate account requirements
rg -n "delegated.*signer\|signer.*delegat" --type ts -i -B 2 -A 2Repository: magicblock-labs/magicblock-engine-examples
Length of output: 68
🏁 Script executed:
# Check if there's a working undelegate example (non-skipped test) that manually constructs accounts
rg -n "undelegateAccounts\|undelegateIx" --type ts -B 5 -A 2Repository: magicblock-labs/magicblock-engine-examples
Length of output: 68
🏁 Script executed:
# Look at the working undelegate test that uses Anchor methods
sed -n '120,144p' roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 870
🏁 Script executed:
# Search for any delegation-related documentation or README
fd -type f \( -name "README*" -o -name "*.md" \) | xargs rg -l "undelegate\|delegation" -i 2>/dev/nullRepository: magicblock-labs/magicblock-engine-examples
Length of output: 258
🏁 Script executed:
# Check for any helper functions that might configure accounts
rg -n "delegatedPayerKeypair\|delegatedAccount" roll-dice/tests/roll-dice-delegated.ts | head -20Repository: magicblock-labs/magicblock-engine-examples
Length of output: 68
🏁 Script executed:
# Look at test setup to see how delegatedPayerKeypair and delegatedAccount are defined
sed -n '1,100p' roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 3860
🏁 Script executed:
# Search for all occurrences of delegatedAccount in the file
grep -n "delegatedAccount\|delegatedPayerKeypair" roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 1761
🏁 Script executed:
# Check if there are any comments or patterns about PDA signing requirements
grep -n "delegation.*PDA\|PDA.*signer" roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 68
🏁 Script executed:
# Look at the complete delegate instruction to see the pattern for comparison
sed -n '95,105p' roll-dice/tests/roll-dice-delegated.tsRepository: magicblock-labs/magicblock-engine-examples
Length of output: 552
delegatedAccount should be marked as isSigner: true.
The delegated account must sign the undelegation instruction. This follows the pattern established in the createDelegateInstruction (line 100) where delegatedPayerKeypair is included as a signer, and is also consistent with the working Anchor-based undelegate test (line 128) where delegatedWallet automatically includes it as a signer. Change line 167 from isSigner: false to isSigner: true.
🤖 Prompt for AI Agents
In roll-dice/tests/roll-dice-delegated.ts around lines 165 to 174, the
delegatedAccount entry in the undelegateAccounts array is incorrectly set with
isSigner: false; update that entry to isSigner: true and ensure the delegated
account's Keypair (delegatedPayerKeypair / delegatedWallet) is included in the
transaction/signers when sending the undelegate instruction so the delegated
account actually signs the instruction.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.