fix(nft): gate the devnet wrapper id behind a devnet feature (#176) - #177
Merged
dcccrypto merged 1 commit intoAug 31, 2026
Merged
Conversation
`PERCOLATOR_DEVNET` compiled unconditionally into every build, including the artifact deployed to mainnet, and was trusted at all three sites that consult the wrapper allowlist. The crate exposed no feature that could exclude it. A program's address is its deploy keypair's public key and is not cluster-scoped, and `DhSkE7u…` is currently unclaimed on mainnet-beta (getAccountInfo returns null). So whoever holds the devnet deploy keypair could deploy an arbitrary program at that address on mainnet and be trusted automatically — able to mint counterfeit Position NFTs from the genuine program, backed by fabricated portfolio accounts they own. Portfolios owned by the real wrapper stay unreachable, so this is counterfeiting rather than a drain. percolator-stake already carries this same allowlist, with the same two program ids, correctly gated (percolator-stake/src/processor.rs:508-523), and its CI builds devnet artifacts with `cargo build-sbf -- --features devnet`. This applies that pattern verbatim: the constant and each allowlist arm move behind `#[cfg(feature = "devnet")]`, using stake's `let is_valid = ...; #[cfg(feature = "devnet")] let is_valid = is_valid || ...;` shadowing. Verified at the artifact level: in a default build the devnet key is absent from the .so entirely — scanning for the eight 4-byte halves SBF uses to materialise a pubkey finds 0 occurrences of each, against 10 for the mainnet id. A `--features devnet` build contains both. CI now runs both feature configurations. The devnet job is load-bearing: without it a misspelled cfg would silently produce a devnet build that trusts nothing, and the crate-wide `#![allow(unexpected_cfgs)]` (lib.rs:5) suppresses the diagnostic that would otherwise catch it. Closes dcccrypto#176 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 7 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
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
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.
Closes #176.
Problem
PERCOLATOR_DEVNETcompiled unconditionally into every build — including the artifact a mainnet deploy produces — and was trusted at all three sites that consult the wrapper allowlist (cpi_v16.rs:98,processor.rs:628,transfer_hook.rs:400). The crate exposed no feature that could exclude it.A program's address is its deploy keypair's public key and is not cluster-scoped.
DhSkE7u…is currently unclaimed on mainnet-beta (getAccountInfo→null) and live on devnet, so whoever holds the devnet deploy keypair could deploy an arbitrary program at that same address on mainnet and be trusted automatically.percolator-prog's own
Cargo.tomlstates the rule this broke, on itsdevnetfeature:Fix
This is not a new design — percolator-stake already carries the identical allowlist, with the identical two program ids, correctly gated (
percolator-stake/src/processor.rs:508-523). The two crates' feature lists differed by exactly this one entry. Applied verbatim, including stake's shadowing idiom:Verification
Artifact level — the devnet key is not in a mainnet build at all. Neither pubkey appears as contiguous bytes in the
.so(SBF materialises them as 32-bitlddwimmediate halves), so scanning for the eight 4-byte halves of each id:[0,0,0,0,0,0,0,0][10,10,10,10,10,10,10,10]--features devnet[3,3,3,3,3,3,3,3][3,3,3,3,3,3,3,3]Behaviour — 4 tests on a default build (devnet rejected at the predicate and through the complete transfer hook, mainnet accepted, an unrelated program rejected) and 3 on
--features devnet(the devnet id trusted again, so devnet deploys keep working). Both controls matter: the mainnet control shows the rejection is specific to the devnet id rather than a harness failing for everything, and the stranger control shows acceptance is allowlist membership rather than a missing check.51 pre-existing tests pass in both configurations.
cargo clippy --all-targets -- -D warningsclean in default, devnet and release.cargo build-sbfsucceeds in both configurations.Coverage completeness —
verify_portfolio_programis the single chokepoint; all 9 call sites route through it, covering every dispatch arm plus the transfer hook and valuation. The crate contains only fourpubkey!literals; the other two (Token-2022, ATA) are cluster-invariant, so there is no second leak of this kind.CI
.github/workflows/test.ymlnow runs both feature configurations. This is load-bearing rather than tidiness: the crate has a crate-wide#, so a misspelled feature name — e.g.#[cfg(feature = "devnett")]— produces no diagnostic at all, not even under-D warnings. It fails closed, but silently. The devnet job catches exactly that, verified by deliberately introducing the typo and watching the devnet test fail.Scope notes
The gate touches three sites, but they are not three independent controls, and the PR should not be read as claiming so:
cpi_v16.rsis the chokepoint;processor.rs:627feeds only adebug_assert!and the crate declares no[profile.release], so debug-assertions are off in the deployed artifact and that line enforces nothing on-chain (:624is the real guard there);transfer_hook.rs:402is defence-in-depth, since the hook performs no CPI post-#105 andpercolator_progis pinned into the ExtraAccountMetaList at mint. Gating all three is still right — they must not disagree.Deliberately not included, and offered as follow-ups:
--all-featuresdefeats the gate silently, and worse, also removes the two#[cfg(not(feature = "devnet"))]rejection tests, so the suite goes 4 → 3 tests and stays green. Nothing in the repo passes--all-featurestoday. A robust guard would be an explicitmainnetfeature plus#[cfg(all(feature = "devnet", feature = "mainnet"))] compile_error!(...), but that adds public feature surface and is a maintainer's call..so— the check tabulated above, ~10 lines. This is the only guard that survives a future refactor of the#[cfg]sites, but it needs a Solana toolchain in CI, which this repo's workflow does not currently install. percolator-stake'sci.yml:92,103is a good template.#![allow(unexpected_cfgs)]to theentrypointmodule rather than the crate. Pre-existing, but newly load-bearing.docs/superpowers/specs/2026-07-20-…-as-built.md§8 and the sibling plan doc) list wrapper and stake flags but mention percolator-nft zero times. Its flags are now cluster-dependent and should be recorded there.Operational note
Once devnet NFTs exist, a devnet deploy built without
--features devnetpermanently strands them: every burn path callsverify_portfolio_program, and only this program's mint-authority PDA can signUnwrapEscrowedPortfolio, so there is no recovery route. That is the reason the build invocation belongs in CI rather than a runbook. There is no such risk today if no devnet NFTs have been minted yet.🤖 Generated with Claude Code