Skip to content

Eliminate per-holding balance events from root claims - #3039

Open
UnArbosSix wants to merge 1 commit into
mainfrom
fix/aggregate-root-claim-settlement
Open

Eliminate per-holding balance events from root claims#3039
UnArbosSix wants to merge 1 commit into
mainfrom
fix/aggregate-root-claim-settlement

Conversation

@UnArbosSix

Copy link
Copy Markdown
Collaborator

Summary

Root claims currently perform one pallet-account transfer per redeemed basket holding—approximately 38 transfers per claim at current basket sizes. This PR replaces that fan-out with silent source debits and one aggregate root-account credit.

Improvement

Operation per claim (N holdings) Before After Improvement at N = 38
Intermediate TAO transfers N 0 38 → 0
Balances.Transfer events N 0 38 → 0
Root-account credits N 1 38 → 1
Source-account debits N N Unchanged; required for reserve backing
Total balance mutations 2N N + 1 76 → 39 (~49% fewer)
Aggregate BasketClaimed events 1 1 Unchanged

At the measured chain rate, root claims generate approximately 1.26 million Balances.Transfer events per day. Removing them is expected to eliminate roughly 30–35 GB/year of event payload.

Correctness

The source debits and aggregate root credit execute transactionally. Claim economics, total issuance, subnet reserve backing, protocol-flow accounting, and rollback behavior remain unchanged.

Testing

  • 1,396 pallet tests passed.
  • 49 root-claim tests passed.
  • Regression coverage verifies balance conservation, zero per-holding Balances events, and one aggregate BasketClaimed event.
  • Strict Clippy, formatting, and diff checks pass.

@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview Aug 4, 2026 3:52pm

Request Review

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

VERY HIGH account-age/activity scrutiny; mitigated by repository write access and substantive merged contributions. No listed Gittensor association; valid branch to main.

The paired exact balance debit and aggregate credit remain inside existing storage transactions, preserving atomicity and issuance while removing only intermediate balance events. Both callers roll back all settlement state if either mutation fails. No AI-review trust-boundary, dependency, origin, panic, or execution-integrity changes were introduced.

Findings

No findings.

Conclusion

No malicious behavior or security vulnerability was found in the static diff review.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

UNKNOWN gittensor association; newer contributor with repository write access and several merged subtensor PRs, so the implementation and stated invariants received independent scrutiny.

The root-claim settlement correctly replaces per-holding transfers with transactional raw debits and one aggregate credit. The paired Unbalanced operations preserve issuance, errors roll back the surrounding storage transaction, and the added test covers balance conservation and event aggregation.

No spec-version auto-fix is needed: local spec_version is 442, ahead of mainnet's 441. The overlapping open PRs concern weight accounting, fee refunds, and SDK migration rather than duplicate implementations.

Findings

Sev File Finding
LOW pallets/subtensor/src/staking/claim_root.rs:1053 Document and test the dissolution event change inline

Conclusion

The primary root-claim optimization is correct and adequately tested. The only concern is a non-blocking, undocumented event change in the shared dissolution path.

@github-actions github-actions Bot left a comment

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.

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

Comment on lines 1050 to +1053
}
};

if let Err(err) = Self::credit_root_account_without_event(tao) {

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.

[LOW] Document and test the dissolution event change

This shared conversion path now also suppresses the Balances::Transfer event during subnet dissolution, although the PR description and new regression coverage discuss root claims only. Because runtime events are externally observable, either preserve the prior transfer behavior here or explicitly document this additional behavior change and add dissolution-path coverage for the intended event set.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

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