Skip to content

Record miner burn as TAO outflow in coinbase#2692

Closed
unconst wants to merge 4 commits into
devnet-readyfrom
unconst/miner-burn-tao-outflow
Closed

Record miner burn as TAO outflow in coinbase#2692
unconst wants to merge 4 commits into
devnet-readyfrom
unconst/miner-burn-tao-outflow

Conversation

@unconst
Copy link
Copy Markdown
Contributor

@unconst unconst commented May 25, 2026

Summary

  • When the coinbase incentive loop encounters an SN-owner-associated hotkey, it skips the payout and recycles or burns the incentive. The corresponding TAO leaving the pool was not being recorded.
  • This PR converts the skipped alpha incentive to its TAO equivalent at the current spot price (no slippage, since no swap occurs) and calls record_tao_outflow(netuid, tao_equivalent) so the subnet's TAO flow accounting stays in sync with what actually leaves circulation on this path.

Code path touched: pallets/subtensor/src/coinbase/run_coinbase.rs, inside the owner_hotkeys.contains(&hotkey) branch of the incentives loop, just before the RecycleOrBurn match.

Test plan

  • cargo check -p pallet-subtensor — clean
  • Existing coinbase tests still pass: cargo test -p pallet-subtensor --lib tests::coinbase
  • Spot-check that SubnetTaoFlow decreases by the expected TAO-equivalent on a subnet where an owner hotkey appears in the incentives list

Made with Cursor

When the SN-owner-hotkey path skips a miner incentive (and the alpha
is then recycled or burned), reflect the corresponding TAO leaving
the pool by calling record_tao_outflow. The skipped incentive is
denominated in alpha, so it is valued at the current spot price for
the outflow accounting (no swap occurs, so no slippage applies).

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/subtensor/src/coinbase/run_coinbase.rs
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: established write-permission contributor; no Gittensor allowlist hit; branch unconst/miner-burn-tao-outflow -> devnet-ready.

The PR touches one pallet runtime file and does not modify .github/ai-review/* or .github/copilot-instructions.md. The latest diff now clamps the computed TAO equivalent to i64::MAX before conversion and records it with Self::record_tao_outflow(netuid, tao_equivalent), addressing the prior Skeptic concern.

Findings

No findings.

Prior-comment reconciliation

  • 838ab442: addressed — The computed tao_equivalent is now passed to Self::record_tao_outflow(netuid, tao_equivalent) after the clamp and conversion.

Conclusion

No malicious intent or security vulnerability found in the current diff. The previous accounting issue is addressed and I found no new panic, overflow, origin-bypass, supply-chain, or trust-file risk.


📜 Previous run (superseded)
Sev File Finding Status
MEDIUM pallets/subtensor/src/coinbase/run_coinbase.rs:629 Record the computed TAO outflow ✅ Addressed
The computed tao_equivalent is now passed to Self::record_tao_outflow(netuid, tao_equivalent) after the clamp and conversion.

🔍 AI Review — Auditor (domain review)

VERDICT: 👎

Write-permission opentensor contributor with substantial prior history; no Gittensor allowlist match found, so standard duplicate-work scrutiny applied.

PR body is substantive and matches the implementation. I checked the overlapping open PRs listed in the pre-fetched context (#2659, #2645, #2638, #2603, #2505); they touch the same file but do not implement this miner-burn TAO-flow accounting, so none is a better duplicate candidate.

No auto-fixes were applied. The devnet spec-version RPC could not be resolved from this environment, so I could not safely determine whether an automatic spec_version bump is required. No cargo tests were run; the blocking issue is visible statically.

Findings

Sev File Finding
HIGH pallets/subtensor/src/coinbase/run_coinbase.rs:630 Avoid per-hotkey SubnetTaoFlow writes in on_initialize inline

Conclusion

The accounting direction looks plausible, but the new storage mutation is inside the coinbase on_initialize incentives loop and is not covered by a flow-accounting test. Please aggregate or explicitly weight the added IO and add targeted coverage before merge.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

JohnReedV and others added 2 commits May 31, 2026 11:17
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/subtensor/src/coinbase/run_coinbase.rs
@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +621 to +630
// Record the miner burn as a TAO outflow. The incentive is
// denominated in alpha, so convert to its TAO equivalent at
// the current spot price (no slippage, since no swap occurs).
let current_price: U96F32 = T::SwapInterface::current_alpha_price(netuid.into());
let tao_equivalent: TaoBalance = current_price
.saturating_mul(asfloat!(incentive))
.saturating_to_num::<u64>()
.min(i64::MAX as u64)
.into();
Self::record_tao_outflow(netuid, tao_equivalent);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[HIGH] Avoid per-hotkey SubnetTaoFlow writes in on_initialize

run_coinbase is reached from the pallet on_initialize hook, which returns a fixed weight. This new record_tao_outflow call mutates SubnetTaoFlow once for every owner-associated hotkey skipped by the incentives loop. OwnedHotkeys is stored as an unbounded Vec, and the skip count is only indirectly bounded by registered UIDs, so this adds loop-amplified DB IO without updating the hook weight.

Aggregate the TAO equivalent into a local total and call record_tao_outflow(netuid, total) once after the incentives loop, or update the hook weight to cover the actual worst case. This should also get a targeted coinbase test asserting SubnetTaoFlow decreases by the expected spot-price TAO equivalent for skipped owner hotkey incentives.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@JohnReedV JohnReedV added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label May 31, 2026
@ppolewicz
Copy link
Copy Markdown
Collaborator

I've seen Ben from taostats catching Const on a conference and Const said this is not going in

@ppolewicz ppolewicz closed this Jun 5, 2026
@ppolewicz ppolewicz deleted the unconst/miner-burn-tao-outflow branch June 5, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants