Add native Rust (non-Anchor) VRF example#78
Open
0xraidr wants to merge 1 commit intomagicblock-labs:mainfrom
Open
Add native Rust (non-Anchor) VRF example#780xraidr wants to merge 1 commit intomagicblock-labs:mainfrom
0xraidr wants to merge 1 commit intomagicblock-labs:mainfrom
Conversation
|
@0xraidr is attempting to deploy a commit to the MagicBlock Labs Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughIntroduces a complete Solana on-chain program for Verifiable Random Function (VRF) integration using the Ephemeral VRF SDK. The implementation includes instruction handlers for player initialization, randomness requests, and VRF callback consumption, alongside comprehensive TypeScript client utilities and integration tests. Totals approximately 1000 lines across Rust program code, test infrastructure, and configuration files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@native-rust-vrf/Cargo.toml`:
- Line 4: Update the Cargo.toml edition setting from "2024" to "2021" so the
crate uses the supported Rust edition for the current Solana/Agave
cargo-build-sbf toolchain; locate the edition = "2024" entry in Cargo.toml and
change it to edition = "2021" to align with other examples and avoid build
failures.
In `@native-rust-vrf/README.md`:
- Line 3: The README currently describes two different dice ranges; update the
documentation to consistently use 1–6 to match the PR intent and the
implementation in callback_consume_randomness.rs: change the intro sentence and
the later occurrence (around line 26) to state randomness is mapped to 1–6, and
verify PlayerState population and any examples/snippets reference the same 1–6
mapping so docs match the logic in callback_consume_randomness.rs.
- Around line 33-36: The README's deploy command wrongly references
reflex_program-keypair.json from another project; update the solana program
deploy example to use a matching placeholder for the keypair (e.g., replace
reflex_program-keypair.json with <your_program>-keypair.json or
native-rust-vrf-keypair.json) so it aligns with the <your_program>.so artifact
and the rest of this crate.
In `@native-rust-vrf/src/entrypoint.rs`:
- Around line 11-25: The entry function process_instruction uses a parameter
named _instruction_data which implies it's unused but it's both logged and
forwarded; rename the parameter to instruction_data in the fn signature and all
uses (the msg! call and the processor::process_instruction invocation) so intent
is clear, and simplify the return by returning the result of
processor::process_instruction(...) directly instead of using ?; Ok(()). Also
gate the verbose msg! that prints the full instruction bytes (around the
existing msg! call) behind a compile-time feature or debug flag (e.g.,
cfg(feature = "verbose_logs") or cfg!(debug_assertions)) to avoid bloating logs
and consuming compute on mainnet.
In `@native-rust-vrf/src/instructions/callback_consume_randomness.rs`:
- Around line 53-55: Remove the stale commented line and stop clamping the
randomness to 1..=6 inside the callback: delete the commented `// p.random_value
= rnd::random_u64(randomness);` and replace the use of
`rnd::random_u8_with_range(randomness, 1, 6)` for `p.random_value` with either
the full random value (e.g., `rnd::random_u64(randomness)`) or store the raw
randomness elsewhere; additionally, to preserve the "dice roll" example without
breaking test polling, add a monotonic field (e.g., `request_id` or `nonce`) to
`PlayerState` and increment it in the same callback so
`pollUntilPlayerRandomValueChanges` can reliably wait on a strictly increasing
field while `p.random_value` can remain a bounded display value if desired;
update references to `p.random_value`, the callback function handling
randomness, and tests that poll `pollUntilPlayerRandomValueChanges` accordingly.
- Around line 42-51: The Borsh try_from_slice is failing if the account buffer
has trailing bytes; update the PlayerState deserialization to explicitly read
only the first PlayerState::LEN bytes: first bind let data =
player.try_borrow_data()?; if data.len() < PlayerState::LEN return
VrfError::PlayerNotInitialized; then call
PlayerState::try_from_slice(&data[..PlayerState::LEN]) to produce p and keep the
existing discriminator check (p.discriminator vs
crate::state::DISCRIMINATOR_PLAYER). This ensures extra allocated bytes won't
break callback_consume_randomness.
In `@native-rust-vrf/src/instructions/initialize_player.rs`:
- Around line 51-76: The current initialize_player logic uses
system_instruction::create_account which fails if the PDA already holds lamports
(owner == system_program::ID) even though you only checked for non-zero lamports
with other owners; update initialize_player to handle the pre-funded-PDA case:
when player_pda.lamports() > 0 && *player_pda.owner == system_program::ID,
instead of calling create_account, perform the allocate + assign + transfer
sequence (allocate space = PlayerState::LEN, assign to *program_id, and transfer
the rent.minimum_balance(PlayerState::LEN) minus existing lamports) using
invoke_signed, or alternatively add a clear comment documenting the precondition
that the PDA must be fully empty before calling create_account; reference the
create_account invocation, the player_pda.lamports() check, PlayerState::LEN,
and the signer seed (state::PLAYER_SEED, authority.key, bump) when implementing
the alternate path so the same signer seeds are used for
allocate/assign/transfer.
In `@native-rust-vrf/src/instructions/request_randomness.rs`:
- Around line 76-81: Add a defensive length check before indexing the borrowed
data: when calling player.try_borrow_data() in request_randomness.rs, ensure
d.len() >= state::PlayerState::LEN (in addition to checking d[0] ==
state::DISCRIMINATOR_PLAYER) and return VrfError::PlayerNotInitialized.into() if
the length is too small; this prevents future panics if the PlayerState layout
grows while still using the existing discriminator check.
- Around line 45-72: The VrfError::AccountOrder variant is being reused for
writability and slot_hashes key mismatches; update the checks to return more
specific errors: replace the oracle_queue.is_writable check to return
VrfError::InvalidAccountFlags, change the slot_hashes key mismatch to return
VrfError::InvalidSlotHashes, and change the program_identity.is_writable and
player.is_writable checks to return VrfError::InvalidAccountFlags (keep
VrfError::AccountOrder strictly for account ordering issues). Add the new enum
variants (InvalidAccountFlags, InvalidSlotHashes) to VrfError and update any
error conversions/tests that expect AccountOrder for these cases.
In `@native-rust-vrf/src/processor.rs`:
- Around line 48-58: Add a sibling assertion in the borsh_tests module to verify
the Borsh encoding of VrfInstruction::InitializePlayer matches the TS fixture;
specifically, in the existing test function
request_randomness_bytes_match_ts_fixture (or a new small test in the same
module) call borsh::to_vec(&VrfInstruction::InitializePlayer).unwrap() and
assert it equals vec![0] so the Rust on-chain encoding is pinned to the
test/initializePlayer.test.ts expectation.
- Around line 40-43: The current comment on the
VrfInstruction::CallbackConsumeRandomness branch is incorrect about how this arm
is reached; update the comment on the CallbackConsumeRandomness match arm to
state that this variant Borsh-encodes to a single byte (e.g., [2]) and that
try_from_slice will reject trailing bytes, so this branch is actually reached
when the caller supplies a Borsh-valid encoding of the enum variant (exactly the
single-byte payload) rather than a malformed 40-byte Borsh payload — replace the
misleading text with this accurate explanation referencing
VrfInstruction::CallbackConsumeRandomness and the try_from_slice decode
behavior.
In `@native-rust-vrf/src/vrf_lite.rs`:
- Around line 15-25: The slice-to-array conversion at the end of
parse_vrf_callback_randomness uses try_into().map_err(...) even though the
preceding length check (against VRF_CALLBACK_IX_LEN) guarantees
instruction_data[8..] is exactly 32 bytes; replace the fallible branch with a
direct, infallible conversion and return Ok(&[u8;32]) (e.g., convert the slice
to an array reference without mapping an unreachable error) inside
parse_vrf_callback_randomness, keeping checks for VRF_CALLBACK_IX_LEN and
CALLBACK_CONSUME_RANDOMNESS unchanged.
In `@native-rust-vrf/test/package.json`:
- Around line 10-15: Update package.json so build/test tools are dev-only:
remove "typescript", "@types/node", and "vitest" from the "dependencies" block
and add them under "devDependencies" instead; while editing, consider bumping
"vitest" to "^4" and add an "engines": { "node": ">=16" } (or your project's
minimal supported Node version) to prevent `@types/node` mismatches and clarify
runtime requirements.
In `@native-rust-vrf/test/utils.ts`:
- Around line 41-43: The helper encodeRequestRandomnessInstruction currently
truncates clientSeed to 8 bits (clientSeed & 0xff) which silently hides invalid
inputs; update encodeRequestRandomnessInstruction to validate that clientSeed is
an integer within 0..255 (and reject negatives/non-integers) and throw a clear
error for out-of-range or non-integer values so callers fail fast; after
validation keep encoding as a single byte (Uint8Array([1, clientSeed])).
- Around line 130-218: Introduce a named discriminator constant (e.g.,
DISCRIMINATOR_PLAYER) and a parsePlayerState(raw: Uint8Array) helper that
validates length against PlayerState::LEN, checks raw[0] ===
DISCRIMINATOR_PLAYER, and returns an object { disc, randomValue: bigint, bump:
number } (or throws on invalid); then replace the raw[0] checks, raw.length
checks, readPlayerRandomU64Le logic, and usages of raw[9] inside
readPlayerRandomU64Le, assertPlayerInitializedForVrf, and
ensurePlayerInitialized to call parsePlayerState and read fields from its result
so validation and offsets are centralized.
In `@native-rust-vrf/test/vrfRequest.test.ts`:
- Around line 125-138: The test is flaky because it polls for
PlayerState.random_value to change (1..=6 can repeat); modify the contract
callback (callback_consume_randomness.rs) to increment a monotonic request
counter (e.g., PlayerState.request_id) each time randomness is consumed, and in
the test replace polling on pollUntilPlayerRandomValueChanges/random_value with
polling for request_id to advance; specifically, update the VRF callback to bump
request_id when setting random_value, and change the test’s poll function (the
anonymous async that calls rpc.getAccountInfo and dataBase64ToBytes) to read and
return request_id, then assert the returned request_id is greater than the
before value instead of comparing random_value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58ed5cb9-eb7a-4b7c-a1d1-d6242aa3fe83
⛔ Files ignored due to path filters (2)
native-rust-vrf/Cargo.lockis excluded by!**/*.locknative-rust-vrf/test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
native-rust-vrf/Cargo.tomlnative-rust-vrf/README.mdnative-rust-vrf/src/entrypoint.rsnative-rust-vrf/src/error.rsnative-rust-vrf/src/instructions/callback_consume_randomness.rsnative-rust-vrf/src/instructions/initialize_player.rsnative-rust-vrf/src/instructions/mod.rsnative-rust-vrf/src/instructions/request_randomness.rsnative-rust-vrf/src/lib.rsnative-rust-vrf/src/processor.rsnative-rust-vrf/src/state.rsnative-rust-vrf/src/vrf_lite.rsnative-rust-vrf/test/initializePlayer.test.tsnative-rust-vrf/test/package.jsonnative-rust-vrf/test/tsconfig.jsonnative-rust-vrf/test/utils.tsnative-rust-vrf/test/vitest.config.tsnative-rust-vrf/test/vrfRequest.test.ts
| [package] | ||
| name = "native-rust-vrf" | ||
| version = "0.1.0" | ||
| edition = "2024" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the current Rust version bundled with the latest Solana / Agave cargo-build-sbf platform-tools, and does it support edition = "2024"?
💡 Result:
The latest Agave / Solana cargo-build-sbf platform-tools uses platform-tools v1.54 (as of March 2026), which bundles a Rust version at least 1.84.1 or newer (v1.47 used 1.84.1; later versions like v1.54 likely use 1.94+ given Agave discussions). It does not support edition = "2024", as Cargo 1.84.0 explicitly lacks edition2024 support (requires Cargo 1.85+ or nightly), and issues persist into late 2025 without evidence of upgrade by April 2026.
Citations:
- 1: https://github.com/anza-xyz/platform-tools/releases
- 2: https://github.com/anza-xyz/platform-tools/releases/tag/v1.54
- 3: v4.0: Bump platform tools version (backport of #11186) anza-xyz/agave#11231
- 4: cargo-build-sbf fails with solana-program 3.0.0 due to missing edition2024 support anza-xyz/agave#8443
- 5: Upgrade Rust to 1.94.0 anza-xyz/agave#11035
- 6: cargo build-sbf rustc version problem anza-xyz/agave#5427
- 7: https://solana.stackexchange.com/questions/17224/how-do-you-get-the-solana-platform-tools-version-or-the-rustc-version-used-by
Downgrade edition = "2024" to edition = "2021".
Edition 2024 is not supported by the latest Solana/Agave cargo-build-sbf platform-tools (v1.54 as of March 2026). The bundled Cargo version lacks edition 2024 support and requires Cargo 1.85+. Contributors will encounter a build failure when running cargo build-sbf. Align with other examples in the repo by using edition = "2021".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/Cargo.toml` at line 4, Update the Cargo.toml edition setting
from "2024" to "2021" so the crate uses the supported Rust edition for the
current Solana/Agave cargo-build-sbf toolchain; locate the edition = "2024"
entry in Cargo.toml and change it to edition = "2021" to align with other
examples and avoid build failures.
| @@ -0,0 +1,58 @@ | |||
| # rust-native-vrf-example | |||
|
|
|||
| Native Solana program (no Anchor) that uses **MagicBlock Ephemeral VRF** via `ephemeral-vrf-sdk`: the user **requests** randomness; the **VRF program** **callbacks** with 32 bytes; you **derive** a value (e.g. 1–6) and store it in `PlayerState`. | |||
Contributor
There was a problem hiding this comment.
Inconsistent dice range between intro and callback section.
Line 3 says randomness is mapped to 1–6 ("roll-dice") while Line 26 says 1–10. Pick one and align with what callback_consume_randomness.rs actually computes — the PR is titled "roll-dice" so 1–6 is the natural choice. Readers copying snippets will otherwise be confused.
Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/README.md` at line 3, The README currently describes two
different dice ranges; update the documentation to consistently use 1–6 to match
the PR intent and the implementation in callback_consume_randomness.rs: change
the intro sentence and the later occurrence (around line 26) to state randomness
is mapped to 1–6, and verify PlayerState population and any examples/snippets
reference the same 1–6 mapping so docs match the logic in
callback_consume_randomness.rs.
Comment on lines
+33
to
+36
| ```bash | ||
| cargo build-sbf | ||
| solana program deploy target/deploy/<your_program>.so --program-id reflex_program-keypair.json | ||
| ``` |
Contributor
There was a problem hiding this comment.
Stray reflex_program-keypair.json from another project.
The deploy command references reflex_program-keypair.json, but nothing else in this PR or crate is named "reflex". This appears copied from an unrelated example and will fail for anyone following the README literally. Use a placeholder consistent with the <your_program>.so style, or native-rust-vrf-keypair.json.
📝 Proposed fix
cargo build-sbf
-solana program deploy target/deploy/<your_program>.so --program-id reflex_program-keypair.json
+solana program deploy target/deploy/native_rust_vrf.so --program-id native-rust-vrf-keypair.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/README.md` around lines 33 - 36, The README's deploy command
wrongly references reflex_program-keypair.json from another project; update the
solana program deploy example to use a matching placeholder for the keypair
(e.g., replace reflex_program-keypair.json with <your_program>-keypair.json or
native-rust-vrf-keypair.json) so it aligns with the <your_program>.so artifact
and the rest of this crate.
Comment on lines
+11
to
+25
| pub fn process_instruction( | ||
| program_id: &Pubkey, | ||
| accounts: &[AccountInfo], | ||
| _instruction_data: &[u8], | ||
| ) -> ProgramResult { | ||
| // Log a message indicating the program ID, number of accounts, and instruction data | ||
| msg!( | ||
| "process_instruction: Program {} is executed with {} account(s) and the following data={:?}", | ||
| program_id, | ||
| accounts.len(), | ||
| _instruction_data | ||
| ); | ||
| processor::process_instruction(program_id, accounts, _instruction_data)?; | ||
| Ok(()) | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Drop the leading underscore on the used parameter, and simplify the return.
_instruction_data is conventionally read as "intentionally unused", but it is in fact passed to both msg! (Line 21) and processor::process_instruction (Line 23). Rename to instruction_data so the signature reflects intent. The trailing ?; Ok(()) is also redundant — just return the inner call. Separately, consider gating the entry msg! (which prints the full instruction byte slice via {:?}) — on a real cluster this consumes CU and bloats logs on every transaction, and other crates in this repo will likely be copy/pasted from this example.
♻️ Proposed fix
pub fn process_instruction(
program_id: &Pubkey,
accounts: &[AccountInfo],
- _instruction_data: &[u8],
+ instruction_data: &[u8],
) -> ProgramResult {
- // Log a message indicating the program ID, number of accounts, and instruction data
- msg!(
- "process_instruction: Program {} is executed with {} account(s) and the following data={:?}",
- program_id,
- accounts.len(),
- _instruction_data
- );
- processor::process_instruction(program_id, accounts, _instruction_data)?;
- Ok(())
+ msg!(
+ "process_instruction: program {} accounts={} data_len={}",
+ program_id,
+ accounts.len(),
+ instruction_data.len()
+ );
+ processor::process_instruction(program_id, accounts, instruction_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.
Suggested change
| pub fn process_instruction( | |
| program_id: &Pubkey, | |
| accounts: &[AccountInfo], | |
| _instruction_data: &[u8], | |
| ) -> ProgramResult { | |
| // Log a message indicating the program ID, number of accounts, and instruction data | |
| msg!( | |
| "process_instruction: Program {} is executed with {} account(s) and the following data={:?}", | |
| program_id, | |
| accounts.len(), | |
| _instruction_data | |
| ); | |
| processor::process_instruction(program_id, accounts, _instruction_data)?; | |
| Ok(()) | |
| } | |
| pub fn process_instruction( | |
| program_id: &Pubkey, | |
| accounts: &[AccountInfo], | |
| instruction_data: &[u8], | |
| ) -> ProgramResult { | |
| msg!( | |
| "process_instruction: program {} accounts={} data_len={}", | |
| program_id, | |
| accounts.len(), | |
| instruction_data.len() | |
| ); | |
| processor::process_instruction(program_id, accounts, instruction_data) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/src/entrypoint.rs` around lines 11 - 25, The entry function
process_instruction uses a parameter named _instruction_data which implies it's
unused but it's both logged and forwarded; rename the parameter to
instruction_data in the fn signature and all uses (the msg! call and the
processor::process_instruction invocation) so intent is clear, and simplify the
return by returning the result of processor::process_instruction(...) directly
instead of using ?; Ok(()). Also gate the verbose msg! that prints the full
instruction bytes (around the existing msg! call) behind a compile-time feature
or debug flag (e.g., cfg(feature = "verbose_logs") or cfg!(debug_assertions)) to
avoid bloating logs and consuming compute on mainnet.
Comment on lines
+42
to
+51
| let mut p = PlayerState::try_from_slice(&player.try_borrow_data()?).map_err(|_| { | ||
| if player.data_is_empty() { | ||
| VrfError::PlayerNotInitialized | ||
| } else { | ||
| VrfError::InvalidPlayerState | ||
| } | ||
| })?; | ||
| if p.discriminator != crate::state::DISCRIMINATOR_PLAYER { | ||
| return Err(VrfError::InvalidPlayerState.into()); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
try_from_slice is strict on trailing bytes — fine today, brittle if PlayerState ever grows or the account is realloc'd.
borsh::BorshDeserialize::try_from_slice rejects input with leftover bytes. Today the PDA is allocated to exactly PlayerState::LEN so this is correct, but if a future migration grows the struct or pre-allocates extra space, every callback will start failing with InvalidPlayerState. Consider being explicit about the slice length:
♻️ Suggested change
- let mut p = PlayerState::try_from_slice(&player.try_borrow_data()?).map_err(|_| {
+ let data_ref = player.try_borrow_data()?;
+ let mut p = PlayerState::try_from_slice(&data_ref[..PlayerState::LEN]).map_err(|_| {
if player.data_is_empty() {
VrfError::PlayerNotInitialized
} else {
VrfError::InvalidPlayerState
}
})?;📝 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.
Suggested change
| let mut p = PlayerState::try_from_slice(&player.try_borrow_data()?).map_err(|_| { | |
| if player.data_is_empty() { | |
| VrfError::PlayerNotInitialized | |
| } else { | |
| VrfError::InvalidPlayerState | |
| } | |
| })?; | |
| if p.discriminator != crate::state::DISCRIMINATOR_PLAYER { | |
| return Err(VrfError::InvalidPlayerState.into()); | |
| } | |
| let data_ref = player.try_borrow_data()?; | |
| let mut p = PlayerState::try_from_slice(&data_ref[..PlayerState::LEN]).map_err(|_| { | |
| if player.data_is_empty() { | |
| VrfError::PlayerNotInitialized | |
| } else { | |
| VrfError::InvalidPlayerState | |
| } | |
| })?; | |
| if p.discriminator != crate::state::DISCRIMINATOR_PLAYER { | |
| return Err(VrfError::InvalidPlayerState.into()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/src/instructions/callback_consume_randomness.rs` around lines
42 - 51, The Borsh try_from_slice is failing if the account buffer has trailing
bytes; update the PlayerState deserialization to explicitly read only the first
PlayerState::LEN bytes: first bind let data = player.try_borrow_data()?; if
data.len() < PlayerState::LEN return VrfError::PlayerNotInitialized; then call
PlayerState::try_from_slice(&data[..PlayerState::LEN]) to produce p and keep the
existing discriminator check (p.discriminator vs
crate::state::DISCRIMINATOR_PLAYER). This ensures extra allocated bytes won't
break callback_consume_randomness.
Comment on lines
+15
to
+25
| pub fn parse_vrf_callback_randomness(instruction_data: &[u8]) -> Result<&[u8; 32], ()> { | ||
| if instruction_data.len() != VRF_CALLBACK_IX_LEN { | ||
| return Err(()); | ||
| } | ||
| if &instruction_data[..8] != CALLBACK_CONSUME_RANDOMNESS.as_ref() { | ||
| return Err(()); | ||
| } | ||
| instruction_data[8..] | ||
| .try_into() | ||
| .map_err(|_| ()) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: simplify the infallible try_into.
After the length check at Line 16 succeeds, instruction_data[8..] is exactly 32 bytes, so the try_into can never produce Err. Minor cleanup:
♻️ Suggested simplification
- instruction_data[8..]
- .try_into()
- .map_err(|_| ())
+ Ok(<&[u8; 32]>::try_from(&instruction_data[8..]).expect("length checked above"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/src/vrf_lite.rs` around lines 15 - 25, The slice-to-array
conversion at the end of parse_vrf_callback_randomness uses
try_into().map_err(...) even though the preceding length check (against
VRF_CALLBACK_IX_LEN) guarantees instruction_data[8..] is exactly 32 bytes;
replace the fallible branch with a direct, infallible conversion and return
Ok(&[u8;32]) (e.g., convert the slice to an array reference without mapping an
unreachable error) inside parse_vrf_callback_randomness, keeping checks for
VRF_CALLBACK_IX_LEN and CALLBACK_CONSUME_RANDOMNESS unchanged.
Comment on lines
+10
to
+15
| "dependencies": { | ||
| "@solana/kit": "^6.8.0", | ||
| "@types/node": "^25.6.0", | ||
| "typescript": "^6.0.3", | ||
| "vitest": "^2.1.9" | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
What is the current latest stable major version of vitest on npm?
💡 Result:
The current latest stable major version of Vitest on npm is 4.
Citations:
- 1: https://www.npmjs.com/package/vitest?activeTab=versions
- 2: https://registry.npmjs.org/vitest
- 3: https://www.npmjs.com/package/vitest
🏁 Script executed:
#!/bin/bash
# Check if there are newer major versions of the other dependencies
# We'll search npm registry info for each package
cat << 'EOF' | python3
import json
import subprocess
packages = [
"@solana/kit",
"@types/node",
"typescript"
]
for pkg in packages:
try:
result = subprocess.run(
["npm", "view", pkg, "versions", "--json"],
capture_output=True,
text=True,
timeout=10
)
if result.returncode == 0:
versions = json.loads(result.stdout)
# Get last 3 versions to show current state
recent = versions[-3:] if len(versions) >= 3 else versions
print(f"{pkg}: recent versions: {recent}")
except Exception as e:
print(f"{pkg}: error - {e}")
EOFRepository: magicblock-labs/magicblock-engine-examples
Length of output: 331
Move test/build tooling to devDependencies.
typescript, @types/node, and vitest are dev-only tools and should not live under dependencies. While this is a private leaf package with no downstream consumer impact, the misclassification will mislead npm install --production, lockfile audits, and tooling that distinguishes runtime vs build deps. Also consider bumping vitest to ^4 (current latest stable) and adding an engines.node field so contributors don't end up with an @types/node major that mismatches their runtime.
♻️ Proposed fix
"scripts": {
"test": "vitest run",
"test:watch": "vitest"
},
"dependencies": {
- "@solana/kit": "^6.8.0",
- "@types/node": "^25.6.0",
- "typescript": "^6.0.3",
- "vitest": "^2.1.9"
- }
+ "@solana/kit": "^6.8.0"
+ },
+ "devDependencies": {
+ "@types/node": "^25.6.0",
+ "typescript": "^6.0.3",
+ "vitest": "^4.0.0"
+ },
+ "engines": {
+ "node": ">=20"
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/test/package.json` around lines 10 - 15, Update package.json
so build/test tools are dev-only: remove "typescript", "@types/node", and
"vitest" from the "dependencies" block and add them under "devDependencies"
instead; while editing, consider bumping "vitest" to "^4" and add an "engines":
{ "node": ">=16" } (or your project's minimal supported Node version) to prevent
`@types/node` mismatches and clarify runtime requirements.
Comment on lines
+41
to
+43
| export function encodeRequestRandomnessInstruction(clientSeed: number): Uint8Array { | ||
| return new Uint8Array([1, clientSeed & 0xff]); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
clientSeed silently truncates to 8 bits.
clientSeed & 0xff accepts any number and quietly drops the high bits — passing 256 produces seed 0, -1 produces 255. Validate the range and reject non-integers so test failures point at the caller, not the wire format.
♻️ Proposed change
export function encodeRequestRandomnessInstruction(clientSeed: number): Uint8Array {
- return new Uint8Array([1, clientSeed & 0xff]);
+ if (!Number.isInteger(clientSeed) || clientSeed < 0 || clientSeed > 0xff) {
+ throw new RangeError(`clientSeed must be a u8 (0..=255), got ${clientSeed}`);
+ }
+ return new Uint8Array([1, clientSeed]);
}📝 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.
Suggested change
| export function encodeRequestRandomnessInstruction(clientSeed: number): Uint8Array { | |
| return new Uint8Array([1, clientSeed & 0xff]); | |
| } | |
| export function encodeRequestRandomnessInstruction(clientSeed: number): Uint8Array { | |
| if (!Number.isInteger(clientSeed) || clientSeed < 0 || clientSeed > 0xff) { | |
| throw new RangeError(`clientSeed must be a u8 (0..=255), got ${clientSeed}`); | |
| } | |
| return new Uint8Array([1, clientSeed]); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/test/utils.ts` around lines 41 - 43, The helper
encodeRequestRandomnessInstruction currently truncates clientSeed to 8 bits
(clientSeed & 0xff) which silently hides invalid inputs; update
encodeRequestRandomnessInstruction to validate that clientSeed is an integer
within 0..255 (and reject negatives/non-integers) and throw a clear error for
out-of-range or non-integer values so callers fail fast; after validation keep
encoding as a single byte (Uint8Array([1, clientSeed])).
Comment on lines
+130
to
+218
| /** | ||
| * `PlayerState` on-chain: disc (u8) + `random_value` (u64 le) + bump (u8). Returns `0n` for empty/bad. | ||
| */ | ||
| export function readPlayerRandomU64Le(accountData: Uint8Array): bigint { | ||
| if (accountData.length < 10) return 0n; | ||
| if (accountData[0] !== 1) return 0n; | ||
| return readU64LE(accountData.subarray(1, 9)); | ||
| } | ||
|
|
||
| export type DevnetKitClients = ReturnType<typeof createDevnetKitClients>; | ||
|
|
||
| /** | ||
| * Fails fast if the player PDA is missing or not `PlayerState`-initialized. Use when you already ran | ||
| * `initialize_player` on-chain and only want VRF tests. | ||
| */ | ||
| export async function assertPlayerInitializedForVrf( | ||
| clients: DevnetKitClients, | ||
| payer: LoadedKeyPairSigner, | ||
| programIdStr: string = getTestProgramId(), | ||
| ): Promise<{ playerPda: Address; bump: number; randomValue: bigint }> { | ||
| const programAddress = address(programIdStr); | ||
| const [playerPda] = await getPlayerPda(programAddress, payer.address); | ||
| const acc = await clients.rpc.getAccountInfo(playerPda, { encoding: "base64" }).send(); | ||
| if (!acc.value) { | ||
| throw new Error( | ||
| "Player PDA missing. Run initialize_player once, or rerun with AUTO_INIT_PLAYER=1.", | ||
| ); | ||
| } | ||
| const raw = dataBase64ToBytes(acc.value.data!); | ||
| if (raw[0] !== 1 || raw.length < 10) { | ||
| throw new Error( | ||
| "Player account is not initialized (expected disc=1). Run initialize_player or AUTO_INIT_PLAYER=1.", | ||
| ); | ||
| } | ||
| return { | ||
| playerPda, | ||
| bump: raw[9]!, | ||
| randomValue: readPlayerRandomU64Le(raw), | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * If the player PDA is missing or not initialized, send `InitializePlayer`. Idempotent for tests. | ||
| */ | ||
| export async function ensurePlayerInitialized( | ||
| clients: DevnetKitClients, | ||
| payer: LoadedKeyPairSigner, | ||
| programIdStr: string = getTestProgramId(), | ||
| ) { | ||
| const { rpc, sendAndConfirmTransaction } = clients; | ||
| const programAddress = address(programIdStr); | ||
| const [playerPda] = await getPlayerPda(programAddress, payer.address); | ||
|
|
||
| const acc = await rpc.getAccountInfo(playerPda, { encoding: "base64" }).send(); | ||
| if (acc.value) { | ||
| const raw = dataBase64ToBytes(acc.value.data!); | ||
| if (raw[0] === 1 && raw.length >= 10) { | ||
| return { playerPda, bump: raw[9]! }; | ||
| } | ||
| } | ||
|
|
||
| const ix = { | ||
| programAddress, | ||
| data: IX_INITIALIZE_PLAYER, | ||
| accounts: [ | ||
| { address: payer.address, role: AccountRole.WRITABLE_SIGNER, signer: payer }, | ||
| { address: playerPda, role: AccountRole.WRITABLE }, | ||
| { address: SYSTEM_PROGRAM, role: AccountRole.READONLY }, | ||
| ], | ||
| }; | ||
| const { value: latest } = await rpc.getLatestBlockhash().send(); | ||
| const message = pipe( | ||
| createTransactionMessage({ version: 0 }), | ||
| (m) => setTransactionMessageFeePayerSigner(payer, m), | ||
| (m) => setTransactionMessageLifetimeUsingBlockhash(latest, m), | ||
| (m) => appendTransactionMessageInstruction(ix, m), | ||
| ); | ||
| const transaction = await signTransactionMessageWithSigners(message, { | ||
| abortSignal: AbortSignal.timeout(30_000), | ||
| }); | ||
| await sendAndConfirmTransaction(asSendableBlockhashTransaction(transaction), { | ||
| commitment: "confirmed", | ||
| }); | ||
|
|
||
| const a2 = await rpc.getAccountInfo(playerPda, { encoding: "base64" }).send(); | ||
| if (!a2.value) throw new Error("player PDA still missing after initialize"); | ||
| const raw2 = dataBase64ToBytes(a2.value.data!); | ||
| return { playerPda, bump: raw2[9]! }; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract a parsePlayerState helper and name the discriminator.
raw[0] !== 1, raw.length < 10, and raw[9] are repeated across readPlayerRandomU64Le, assertPlayerInitializedForVrf, and ensurePlayerInitialized. A named constant (mirroring state::DISCRIMINATOR_PLAYER and PlayerState::LEN) plus one decode helper removes the drift risk if PlayerState ever grows a field.
♻️ Sketch
+const PLAYER_DISCRIMINATOR = 1;
+const PLAYER_STATE_LEN = 10; // disc(u8) + random_value(u64) + bump(u8)
+
+function parsePlayerState(raw: Uint8Array): { randomValue: bigint; bump: number } | null {
+ if (raw.length < PLAYER_STATE_LEN || raw[0] !== PLAYER_DISCRIMINATOR) return null;
+ return { randomValue: readU64LE(raw.subarray(1, 9)), bump: raw[9]! };
+}…then use parsePlayerState inside readPlayerRandomU64Le, assertPlayerInitializedForVrf, and ensurePlayerInitialized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/test/utils.ts` around lines 130 - 218, Introduce a named
discriminator constant (e.g., DISCRIMINATOR_PLAYER) and a parsePlayerState(raw:
Uint8Array) helper that validates length against PlayerState::LEN, checks raw[0]
=== DISCRIMINATOR_PLAYER, and returns an object { disc, randomValue: bigint,
bump: number } (or throws on invalid); then replace the raw[0] checks,
raw.length checks, readPlayerRandomU64Le logic, and usages of raw[9] inside
readPlayerRandomU64Le, assertPlayerInitializedForVrf, and
ensurePlayerInitialized to call parsePlayerState and read fields from its result
so validation and offsets are centralized.
Comment on lines
+125
to
+138
| const afterRandom = await pollUntilPlayerRandomValueChanges( | ||
| async () => { | ||
| const a = await rpc.getAccountInfo(playerPda, { encoding: "base64" }).send(); | ||
| if (!a.value) return null; | ||
| return dataBase64ToBytes(a.value.data!); | ||
| }, | ||
| beforeRandom, | ||
| { maxWaitMs: 60_000, intervalMs: 500 }, | ||
| ); | ||
|
|
||
| expect(afterRandom).not.toBe(beforeRandom); | ||
| console.log("PlayerState.random_value (u64 LE) after VRF callback:", afterRandom.toString()); | ||
| console.log("hex:", "0x" + afterRandom.toString(16)); | ||
| }); |
Contributor
There was a problem hiding this comment.
Flaky assertion: ~1/6 chance the new roll equals the previous one.
callback_consume_randomness.rs (Line 54) sets random_value to a roll in 1..=6. If the test is re-run on an already-initialized player whose stored value is also in 1..=6, there is a ~16% chance the new VRF roll matches the prior one, and pollUntilPlayerRandomValueChanges will time out even though the callback succeeded. The very first run is safe (initial value is 0, roll is 1..=6), but repeated CI runs against the same wallet will be flaky.
Consider asserting on a request "version"/counter (e.g., bump a request_id in PlayerState inside the callback) and polling for that to advance, rather than for the dice value itself to differ.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@native-rust-vrf/test/vrfRequest.test.ts` around lines 125 - 138, The test is
flaky because it polls for PlayerState.random_value to change (1..=6 can
repeat); modify the contract callback (callback_consume_randomness.rs) to
increment a monotonic request counter (e.g., PlayerState.request_id) each time
randomness is consumed, and in the test replace polling on
pollUntilPlayerRandomValueChanges/random_value with polling for request_id to
advance; specifically, update the VRF callback to bump request_id when setting
random_value, and change the test’s poll function (the anonymous async that
calls rpc.getAccountInfo and dataBase64ToBytes) to read and return request_id,
then assert the returned request_id is greater than the before value instead of
comparing random_value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The existing MagicBlock VRF "roll-dice" example is built on Anchor and pairs with the Anchor client. This adds a plain Solana program (solana-program + Borsh + ephemeral-vrf-sdk) and @solana/kit integration tests on devnet, so users can see the same VRF flow without Anchor in the on-chain or client stack.
Summary by CodeRabbit
New Features
Documentation
Tests