Improve Benchmarking by JohnReedV · Pull Request #2798 · RaoFoundation/subtensor

JohnReedV · 2026-06-25T16:23:21Z

Description

This PR

Modifies the benchmark flow so that only extrinsics that have drifted passed the threshold or have incorrect read/writes are updated rather than replacing the entire file and creating conflicts everywhere.
Adds a lint to check for extrinsics with missing benchmarks. This can be hidden with
#[allow(unknown_lints, benchmarked_weight_not_plugged)]
Adds 48 missing benchmarks
Fixes a bug where benchmarks could fail on noisy internal component drift even when the actual benchmark-level weight was still within the configured threshold.
Adds block_step to the benchmark flow so the on_initialize weight is generated instead of hardcoded. The benchmark simulates the current worst case with 2 epochs in a block.
Updates the drift check to include proof size and to compare the actual benchmark-level weight instead of failing on noisy internal component drift.

github-actions · 2026-06-25T16:25:44Z

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

Baseline-to-medium scrutiny: established OpenTensor-associated contributor with substantial subtensor history; cached repo permission is read; no Gittensor association found; final commits include non-PR-author activity by UnArbosSix; branch apply-benchmark-to-only-bad-benches -> devnet-ready.

The PR does not modify .github/ai-review/* or .github/copilot-instructions.md. Manifest edits only enable features on existing workspace dependencies; no new external dependency or Cargo.lock change is introduced.

The latest revision fills the register_limit subnet, but the benchmark still pre-runs the expensive prune scan before the measured call, so the prior underweight concern remains.

Findings

Sev	File	Finding
HIGH	`pallets/subtensor/src/benchmarks.rs:3090`	Benchmark the full-subnet registration path	inline

Prior-comment reconciliation

2caec2f2: not addressed — The subnet is now populated to MaxAllowedUids, but the benchmark calls get_neuron_to_prune before the measured register_limit dispatch. That warms the full-subnet prune scan and the generated weight still omits the scan cost, so the undercharge remains.

Conclusion

The PR appears legitimate, but register_limit can still be generated below its full-subnet production cost because the benchmark warms the prune path during setup. That leaves a public registration path undercharged.

📜 Previous run (superseded)

~~Sev~~	~~File~~	~~Finding~~	Status
~~HIGH~~	~~`pallets/subtensor/src/benchmarks.rs:3080`~~	~~Benchmark the full-subnet registration path~~	➡️ Carried forward to current findings The subnet is now populated to `MaxAllowedUids`, but the benchmark calls `get_neuron_to_prune` before the measured `register_limit` dispatch. That warms the full-subnet prune scan and the generated weight still omits the scan cost, so the undercharge remains.

🔍 AI Review — Auditor (domain review)

VERDICT: 👎

Gittensor UNKNOWN; author has repo write permission, @opentensor profile context, and substantial recent subtensor history, so calibrated as established-contributor domain review.

PR body is substantive and matches the benchmark/tooling scope, so I would leave it unchanged.

Validation performed: bash -n scripts/benchmark_action.sh passed, and git diff --check origin/devnet-ready...HEAD passed. I did not run cargo tests because the blocking issue is visible from the benchmark setup and dispatch path.

Findings

Sev	File	Finding
HIGH	`pallets/subtensor/src/benchmarks.rs:3080`	Benchmark the full-subnet registration path	inline

Prior-comment reconciliation

21660033: addressed — No conflict markers remain in pallets/proxy/src/weights.rs, and git diff --check origin/devnet-ready...HEAD passes.
ece3dc4d: addressed — No conflict markers remain in pallets/utility/src/weights.rs, and git diff --check origin/devnet-ready...HEAD passes.
2cfa55a3: addressed — The WeightInfo matcher still requires a method-name boundary and retains regression coverage for prefix false positives such as swap_coldkey vs swap_coldkey_announced.

Conclusion

Blocking on the register_limit benchmark: the PR plugs this dispatch into generated weights, but the benchmark measures only the cheap append path while production can take the full-subnet replacement path. That can leave the generated weight materially below the worst-case execution cost.

github-actions · 2026-06-25T16:28:28Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

github-actions · 2026-06-25T17:53:38Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

github-actions · 2026-06-25T18:52:27Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

github-actions · 2026-06-26T16:47:33Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

github-actions · 2026-06-26T17:17:22Z

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

…tensor/subtensor into apply-benchmark-to-only-bad-benches

github-actions