Skip to content

Conversation

@sporicle
Copy link

@sporicle sporicle commented Nov 18, 2025

Summary by CodeRabbit

  • Refactor
    • Reorganized SDK internal module structure to support feature-gated type compatibility
    • Updated internal type handling for improved integration flexibility across different build configurations

@coderabbitai
Copy link

coderabbitai bot commented Nov 18, 2025

Walkthrough

This PR introduces a feature-gated abstraction layer for Pubkey types. When the "anchor" feature is enabled, the SDK uses anchor_lang::prelude::Pubkey; otherwise, it uses solana_program::pubkey::Pubkey. The Id trait implementations and public API signatures are updated to use this unified alias.

Changes

Cohort / File(s) Summary
Feature-gated Pubkey abstraction
sdk/src/lib.rs
Introduces conditional compilation with cfg(not(feature = "anchor")) and cfg(feature = "anchor") blocks that create a private solana module re-exporting the appropriate Pubkey and system_program types. Exposes pub use solana::Pubkey as the unified alias and updates the id() function signature to return this aliased type instead of the explicit ::solana_program::pubkey::Pubkey.
Id trait implementation update
sdk/src/anchor.rs
Updates the VrfProgram implementation of the Id trait to return anchor_lang::prelude::Pubkey instead of ::solana_program::pubkey::Pubkey. The function body remains unchanged, still returning crate::consts::VRF_PROGRAM_ID.
Import refactoring
sdk/src/instructions.rs
Removes direct import of Pubkey from solana_program and adds imports from crate::solana. Replaces the hardcoded path solana_program::system_program::ID with system_program::ID in the AccountMeta::new_readonly call for the system program account.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify feature gate logic is correctly implemented for both anchor and non-anchor scenarios
  • Confirm that all public API changes (Id trait return type, id() function signature, Pubkey re-export) are intentional and maintain backward compatibility where applicable
  • Ensure type consistency across the three files and that callers can adapt to the aliased Pubkey type
  • Validate that the private solana module correctly re-exports the appropriate types based on feature presence

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: support for anchor 0.32.1' directly aligns with the changeset, which updates the codebase to support a specific version of the Anchor framework through feature-gated conditional compilation and type adjustments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch anchor-0.32.1

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cd77ec and 726e6f4.

📒 Files selected for processing (2)
  • sdk/src/instructions.rs (2 hunks)
  • sdk/src/lib.rs (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
sdk/src/lib.rs (1)
sdk/src/anchor.rs (1)
  • id (12-14)
⏰ 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: lint
🔇 Additional comments (2)
sdk/src/instructions.rs (1)

2-2: Instruction wiring correctly aligned with new solana alias

Switching to crate::solana::{system_program, Pubkey} and system_program::ID cleanly plugs this module into the new feature-gated alias without changing the instruction’s behavior or account layout. Looks good.

Also applies to: 29-29

sdk/src/lib.rs (1)

9-23: Verify both feature configurations compile; note inconsistent Pubkey imports across modules

The cfg-gated solana module structure in lib.rs is syntactically correct and the re-export pattern is sound. However, inspection reveals an inconsistency: while instructions.rs correctly imports use crate::solana::{system_program, Pubkey}, the modules types.rs, consts.rs, and pda.rs all import Pubkey directly from solana_program::pubkey::Pubkey rather than from the unified re-export. This mixed approach works (since anchor_lang::prelude::Pubkey is an alias to solana_program::pubkey::Pubkey), but creates import path inconsistency.

The cargo check verification could not run in the sandbox environment. Please manually verify both feature configurations compile:

cargo check --no-default-features
cargo check --features anchor

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 933683f and 98664aa.

📒 Files selected for processing (1)
  • sdk/Cargo.toml (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run Tests
sdk/Cargo.toml

[error] 1-1: Cargo workspace manifest parse error: feature 'anchor' includes 'anchor-lang', but 'anchor-lang' is not an optional dependency. A non-optional dependency of the same name is defined; consider adding 'optional = true' to its definition.


[features]
default = []
anchor = ["anchor-lang"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Resolve Cargo manifest conflict between feature gate and dependency requirement.

The feature definition on line 16 references anchor-lang, but anchor-lang on line 19 is defined as a required (non-optional) dependency. Cargo requires that feature gates reference only optional dependencies, causing the pipeline to fail with a manifest parse error.

Resolve this by choosing one of the following approaches:

Option 1: Keep anchor-lang as an optional dependency (if the feature gating is intentional)

Restore optional = true to the dependency:

-anchor-lang = { workspace = true }
+anchor-lang = { workspace = true, optional = true }

Option 2: Remove the feature gate (if anchor-lang is truly required)

Remove the feature definition since anchor-lang is now unconditionally required:

 [features]
 default = []
-anchor = ["anchor-lang"]

Verify which approach aligns with the PR's intent to support Anchor 0.32.1.

Also applies to: 19-19


I'll verify the actual state of the Cargo.toml file and the configuration.




🏁 Script executed:

#!/bin/bash
# Check the actual state of sdk/Cargo.toml, especially lines around 16-19
cat sdk/Cargo.toml

Length of output: 575


🏁 Script executed:

#!/bin/bash
# Also search for anchor-lang across the file to understand full context
grep -n "anchor" sdk/Cargo.toml

Length of output: 65


🏁 Script executed:

#!/bin/bash
# Check if there are any other references to optional dependencies or features
grep -n -A2 -B2 "\[features\]" sdk/Cargo.toml

Length of output: 80


🏁 Script executed:

#!/bin/bash
# Look for dependencies section
grep -n -A10 "\[dependencies\]" sdk/Cargo.toml

Length of output: 182



---

🌐 Web query:

```web
Cargo feature gates optional dependencies requirement

💡 Result:

Short summary:

  • Mark a dependency optional with optional = true in Cargo.toml; Cargo will (currently) create an implicit feature with the same name that enables that optional dep. Use that feature in code with #[cfg(feature = "name")]. [1][2]

  • To expose an optional dependency explicitly as a feature (or to make feature groups activate a dependency), list the dependency name in a feature entry or use the dep: syntax: e.g.
    [features]
    my-feature = ["dep:foo"]
    or in older style the implicit foo = ["dep:foo"]. This is how you gate inclusion of that optional crate. [1][2]

  • Cargo also supports required-features on targets (binaries/examples/bench/tests) to disable building a target unless features are enabled (useful for feature-gated binaries). See the "required-features" docs. [1][3]

  • Note: an RFC removed implicit exposure of optional dependencies as plain features (authors should prefer dep:... when they intend to expose them). Check the Cargo/features docs or RFC 3491 for current behavior if you need exact, up-to-date semantics. [3]

If you want, tell me your Cargo.toml and I’ll show the exact manifest lines you need.

Sources: Cargo features documentation and RFC. [1] [2] [3]

References:
[1] Cargo Book — Features & Optional dependencies (Rust docs).
[2] Cargo Book — Manifest / features rules (Cargo reference).
[3] RFC 3491 (remove implicit features).


Critical: Resolve Cargo manifest conflict—feature references non-optional dependency.

The feature definition on line 16 references anchor-lang, but anchor-lang on line 19 is not marked as an optional dependency. In Cargo, features can only reference optional dependencies, causing the manifest parse error.

Resolve by choosing one approach:

Option 1: Make anchor-lang optional (if feature gating is intentional)

Add optional = true and use the dep: syntax per current Cargo best practices:

-anchor-lang = { workspace = true }
+anchor-lang = { workspace = true, optional = true }
-anchor = ["anchor-lang"]
+anchor = ["dep:anchor-lang"]

Option 2: Remove the feature gate (if anchor-lang is truly required)

 [features]
 default = []
-anchor = ["anchor-lang"]

Verify the intent and apply the appropriate fix.

📝 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
anchor = ["anchor-lang"]
anchor-lang = { workspace = true, optional = true }
Suggested change
anchor = ["anchor-lang"]
anchor = ["dep:anchor-lang"]
🤖 Prompt for AI Agents
In sdk/Cargo.toml around line 16, the feature "anchor = [\"anchor-lang\"]"
references a non-optional dependency which causes a manifest error; fix by
either 1) making anchor-lang optional and updating the feature to reference the
dependency via the dep: syntax (set anchor-lang = { version = \"...\", optional
= true } in [dependencies] and feature = [\"dep:anchor-lang\"]) if you intended
feature-gating, or 2) remove the feature reference entirely and keep anchor-lang
as a normal required dependency if it should always be present.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98664aa and 1cd77ec.

📒 Files selected for processing (1)
  • sdk/Cargo.toml (1 hunks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants