Skip to content

fix: lock error codes, refresh toolchain drift, add scanner emergency pause#22

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1779965793-rigorous-bug-scan-fixes
Open

fix: lock error codes, refresh toolchain drift, add scanner emergency pause#22
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1779965793-rigorous-bug-scan-fixes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented May 28, 2026

Copy link
Copy Markdown

Why

A rigorous scan of the workspace surfaced several correctness and consistency issues that should land before any further on-chain work. The headline item is a silent error-code numbering bug in GraveVault that would have broken every downstream consumer (indexer parsers, SDK error matchers, IDL clients). The rest is governance/operations gaps, a CI install bug that masked the anchor build step on main, and toolchain documentation drift.

What

  • GraveVault: error codes now match the documented 7000..=7014 range. Anchor's #[error_code] macro applies a default +6000 offset on top of every Rust discriminant. With the previous Unauthorized = 7000, ... discriminants, the on-chain values emitted were actually 13000..=13014 — silently wrong. Fix overrides the offset (#[error_code(offset = 7000)]) and renumbers discriminants to 0..=14. A new unit test in both programs/grave-scanner/src/errors.rs and programs/grave-vault/src/errors.rs locks the on-chain numbering against future drift.
  • GraveScanner: add emergency_pause instruction. ProtocolConfig.paused was checked inside evaluate_pool_phase_1 / _phase_2 but no instruction could set it — the kill switch was unreachable. New instruction is multisig-only, immediate, and emits ProtocolPauseChanged. Mirrors GraveVault's emergency_pause. update_protocol_config also gains lp_burn_dust_threshold as a tunable parameter (the field's doc-comment said it was governance-tunable but the handler never touched it).
  • SDK: priorityFee.ts no longer truncates BN inputs. computeOperationalMaxLamportsPerCu was calling BN.toNumber() on lamport amounts, which throws for values above Number.MAX_SAFE_INTEGER (~9.0e15) — well within reach for realistic Solana profits. Math now runs in pure BN via a 10_000-bps integer quantisation of marginRatio, plus tighter input validation (rejects NaN, negative profit).
  • CI: anchor build was reliably broken on main. cargo binstall --no-confirm anchor-cli was reporting "already installed" from Swatinem/rust-cache-restored ~/.cargo/.crates2.json while ~/.cargo/bin/anchor was missing, so the subsequent build step crashed with command not found (exit 127). Add --force to binstall so it always installs the binary regardless of cached metadata. Verified the same failure exists on main (e.g. run 25846568599) — so this commit fixes a preexisting CI bug, not one introduced by this PR.
  • Toolchain drift: README.md, CONTRIBUTING.md, scripts/check-toolchain.sh, .github/dependabot.yml, and inline comments in both program crates were still citing the pre-3.x stack (Solana 2.0.x / Anchor 0.31.x / Rust 1.79+ / Node 20 LTS) while Anchor.toml, rust-toolchain.toml, and package.json had already moved to Solana 3.0.10 / Anchor 0.32.1 / Rust 1.91.1 / Node 24 LTS. Aligned everything; check-toolchain.sh now passes locally with the actual stack. Also bumps Cargo.lock to track anchor-attribute-* = 0.32.1 (the workspace was on 0.32.1 in Cargo.toml but the lockfile had stale 0.31.1 entries).

Risk

  • Charter invariants touched: None. The 20% ceiling, lp_holder_pool_vault unsweepability, and 72h timelock are all unchanged. The new emergency_pause follows the same Charter framing as GraveVault's ("Emergency pause is immediate") and is multisig-only.
  • Migration required: No on-chain migration. The GraveVault error-code fix is pre-mainnet — there are no deployed instances whose IDL or stored discriminants need to be migrated. UpdateProtocolConfigParams gains a new Option<u64> field (lp_burn_dust_threshold), so existing serialized payloads need to be regenerated by clients before being re-submitted — also a non-issue pre-mainnet.
  • Audit surface: New emergency_pause instruction on GraveScanner (same accounts shape as GraveVault's equivalent, no CPIs, single bool write + event emit). New optional parameter in update_protocol_config. Error-code numbering reshuffled for GraveVault (variants and #[msg] strings unchanged; only on-chain discriminants change). No new accounts, no new PDAs, no new CPIs.

Tests

  • cargo fmt --all -- --check (CI green)
  • cargo clippy --all-targets -- -D warnings (CI green)
  • anchor build (CI green — see the CI fix above)
  • pnpm -r typecheck (CI green)
  • anchor test (no integration tests exist yet per tests/README.md)
  • bash scripts/terminology-lint.sh (CI green)
  • cargo test --workspace --lib — 22 tests pass (20 existing + 2 new on_chain_codes_match_docs)
  • Hand-validated computeOperationalMaxLamportsPerCu against the old buggy BN.toNumber() overflow path and against ceiling-clamp / margin=0 / NaN / negative-profit edge cases

Checklist

  • Commits are signed (git commit -S). (Signing key not configured in this environment; verify on merge or sign locally.)
  • Branch follows feat/, fix/, chore/, or docs/ prefix. (devin/<ts>-rigorous-bug-scan-fixes per the repo's session-naming convention.)
  • Conventional Commit messages.
  • Spec changes are reflected in docs/ in the same PR. (docs/error_codes.md already documented 7000..=7014 — the code now matches; toolchain docs in README/CONTRIBUTING aligned to the actual pinned stack.)
  • Canonical v4.0 terminology only — terminology lint passes.

Link to Devin session: https://app.devin.ai/sessions/73cd25de45e649ac9e00ac72d759492d
Requested by: @graveyieldprotocol

…r emergency pause

Several correctness and consistency issues found during a rigorous scan.

* GraveVault error codes were silently emitting on-chain values 13000..=13014
  instead of the documented 7000..=7014 range. Anchor's `#[error_code]`
  macro applies a default +6000 offset on top of every Rust discriminant, so
  the explicit `= 7000` discriminants compounded to (7000 + 6000). Fix:
  override the offset (`#[error_code(offset = 7000)]`) and renumber the
  discriminants to 0..=14. Add a unit test that locks down the on-chain
  numbering for both programs against future drift.

* GraveScanner stored a `paused` flag and gated `evaluate_pool_*` on it but
  had no instruction to set it. Add an `emergency_pause` instruction
  modelled on GraveVault's: multisig-only, immediate, emits
  `ProtocolPauseChanged`.

* GraveScanner `update_protocol_config` was missing
  `lp_burn_dust_threshold` as a tunable parameter — set at `initialize`
  and then frozen for the lifetime of the deployment, despite the field's
  doc-comment claiming it was governance-tunable. Add it as an optional
  param.

* sdk/src/priorityFee.ts called `BN.toNumber()` on a lamport amount which
  throws for values above `Number.MAX_SAFE_INTEGER` (~9e15). Rewrite the
  margin scaling in pure BN math via a 10_000-bps integer quantisation,
  and tighten input validation (NaN, negative profit).

* Toolchain drift: README.md, CONTRIBUTING.md, scripts/check-toolchain.sh,
  .github/dependabot.yml, and various rust source comments still cited the
  pre-3.x stack (Solana 2.0.x / Anchor 0.31.x / Rust 1.79+ / Node 20 LTS)
  while Anchor.toml, rust-toolchain.toml, and package.json had already
  moved to Solana 3.0.10 / Anchor 0.32.1 / Rust 1.91.1 / Node 24 LTS.
  Align the docs and toolchain checker.

* Cargo.lock was inconsistent with Cargo.toml — `anchor-attribute-*` were
  still locked at 0.31.1. Local builds refreshed the lockfile to 0.32.1
  matching workspace deps.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

…mask a missing binary

Swatinem/rust-cache restores ~/.cargo/.crates2.json (binstall's
"is-installed" registry) but does NOT restore ~/.cargo/bin/. Without
`--force`, binstall finds the cached metadata, prints "already
installed", and exits without installing the binary, leaving the
subsequent `anchor build` step with command not found (exit 127).

Reproduces deterministically on the `anchor build` job — see
the previous failed run on main (e.g. job 75943039464) for the same
"already installed" + "command not found" sequence.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.

1 participant