fix: lock error codes, refresh toolchain drift, add scanner emergency pause#22
Open
devin-ai-integration[bot] wants to merge 2 commits into
Open
fix: lock error codes, refresh toolchain drift, add scanner emergency pause#22devin-ai-integration[bot] wants to merge 2 commits into
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
…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>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…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>
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.
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 buildstep onmain, and toolchain documentation drift.What
#[error_code]macro applies a default+6000offset on top of every Rust discriminant. With the previousUnauthorized = 7000, ...discriminants, the on-chain values emitted were actually13000..=13014— silently wrong. Fix overrides the offset (#[error_code(offset = 7000)]) and renumbers discriminants to0..=14. A new unit test in bothprograms/grave-scanner/src/errors.rsandprograms/grave-vault/src/errors.rslocks the on-chain numbering against future drift.emergency_pauseinstruction.ProtocolConfig.pausedwas checked insideevaluate_pool_phase_1/_phase_2but no instruction could set it — the kill switch was unreachable. New instruction is multisig-only, immediate, and emitsProtocolPauseChanged. Mirrors GraveVault'semergency_pause.update_protocol_configalso gainslp_burn_dust_thresholdas a tunable parameter (the field's doc-comment said it was governance-tunable but the handler never touched it).priorityFee.tsno longer truncates BN inputs.computeOperationalMaxLamportsPerCuwas callingBN.toNumber()on lamport amounts, which throws for values aboveNumber.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 ofmarginRatio, plus tighter input validation (rejects NaN, negative profit).anchor buildwas reliably broken onmain.cargo binstall --no-confirm anchor-cliwas reporting "already installed" fromSwatinem/rust-cache-restored~/.cargo/.crates2.jsonwhile~/.cargo/bin/anchorwas missing, so the subsequent build step crashed withcommand not found(exit 127). Add--forceto binstall so it always installs the binary regardless of cached metadata. Verified the same failure exists onmain(e.g. run 25846568599) — so this commit fixes a preexisting CI bug, not one introduced by this PR.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) whileAnchor.toml,rust-toolchain.toml, andpackage.jsonhad already moved to Solana 3.0.10 / Anchor 0.32.1 / Rust 1.91.1 / Node 24 LTS. Aligned everything;check-toolchain.shnow passes locally with the actual stack. Also bumpsCargo.lockto trackanchor-attribute-* = 0.32.1(the workspace was on 0.32.1 inCargo.tomlbut the lockfile had stale 0.31.1 entries).Risk
lp_holder_pool_vaultunsweepability, and 72h timelock are all unchanged. The newemergency_pausefollows the same Charter framing as GraveVault's ("Emergency pause is immediate") and is multisig-only.UpdateProtocolConfigParamsgains a newOption<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.emergency_pauseinstruction on GraveScanner (same accounts shape as GraveVault's equivalent, no CPIs, single bool write + event emit). New optional parameter inupdate_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 pertests/README.md)bash scripts/terminology-lint.sh(CI green)cargo test --workspace --lib— 22 tests pass (20 existing + 2 newon_chain_codes_match_docs)computeOperationalMaxLamportsPerCuagainst the old buggyBN.toNumber()overflow path and against ceiling-clamp / margin=0 / NaN / negative-profit edge casesChecklist
git commit -S). (Signing key not configured in this environment; verify on merge or sign locally.)feat/,fix/,chore/, ordocs/prefix. (devin/<ts>-rigorous-bug-scan-fixesper the repo's session-naming convention.)docs/in the same PR. (docs/error_codes.mdalready documented 7000..=7014 — the code now matches; toolchain docs in README/CONTRIBUTING aligned to the actual pinned stack.)Link to Devin session: https://app.devin.ai/sessions/73cd25de45e649ac9e00ac72d759492d
Requested by: @graveyieldprotocol