Skip to content

fix(dao-vote-delegation): scale delegated VP on same-percent refresh (audit Critical 1) - #933

Open
noahsaso wants to merge 2 commits into
developmentfrom
fix-delegation-refresh-weight
Open

fix(dao-vote-delegation): scale delegated VP on same-percent refresh (audit Critical 1)#933
noahsaso wants to merge 2 commits into
developmentfrom
fix-delegation-refresh-weight

Conversation

@noahsaso

@noahsaso noahsaso commented Sep 1, 2026

Copy link
Copy Markdown
Member

Fixes audit Finding 1 (Critical): "A delegation refresh can credit a delegate with the delegator's full weight rather than the delegated share". Also closes the other face of the same defect, Finding 5 (High): "The same refresh can instead destroy a co-delegator's voting power".

Root cause

When a delegator re-delegates to the same delegate with the same percent before the delegation expires, handle_redelegation only moves the scheduled expiration. It called update_delegated_vp_expiration with the delegator's raw voting power instead of the delegated share (calculate_delegated_vp(vp, percent)), while every other path (new delegation, undelegation, percent change, voting power change hooks) uses the scaled amount.

  • With an existing expiration, the refresh cancelled the scheduled scaled decrement with a raw increment and scheduled a raw decrement at the new expiration, so from the original expiration onward the delegate was credited with the delegator's entire weight (Finding 1).
  • With no existing expiration (delegation created while expiry was disabled, refreshed after the DAO enabled it), the raw decrement at the new expiration was unpaired and removed other delegators' contributions from the delegate's running total, or underflowed (Finding 5).

At a 100% delegation the two amounts coincide, which is why the existing test_expiration_update test never caught it.

Fix

  • Pass calculate_delegated_vp(delegator_vp, new_percent) on the same-percent refresh path.
  • Rename the amount parameters of add_delegated_vp, remove_delegated_vp_if_not_expired and update_delegated_vp_expiration from vp to delegated_vp, and the raw-power argument of handle_redelegation to delegator_vp, so the two quantities can no longer be confused (as recommended by the audit).

Tests

Three new tests, all of which fail on development and pass with the fix:

  • test_same_percent_refresh_moves_only_delegated_voting_power: 50% refresh with a co-delegator; only the refreshing delegator's scaled share moves to the new expiration.
  • test_same_percent_refresh_after_enabling_expiration_preserves_co_delegator_vp: the Finding 5 scenario (delegation created with expiry disabled, DAO enables expiry, delegator refreshes); the co-delegator's non-expiring contribution survives.
  • test_mixed_percent_refreshes_keep_total_equal_to_sum_of_scaled_delegations: four delegators with different weights and percents (60/50/75/100%), two refreshed mid-period; the delegate's total equals the sum of scaled live delegations at every expiration boundary.

Verified locally: cargo test -p dao-vote-delegation (51 passed), cargo +nightly-2024-01-08 fmt --all -- --check, cargo +nightly-2024-01-08 clippy --all-targets -- -D warnings.

Not included (audit follow-ups)

  • Finding 5 also recommends replacing the unchecked subtraction in cw-wormhole::decrement with a checked one so an accounting inconsistency surfaces as a named error rather than a trap. That is a separate package change and is left for a follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YBSNZfoCq2XqymbVV9r5rE

tauagent and others added 2 commits September 1, 2026 19:05
Cover the audit's Finding 5 scenario (a delegation created while expiry was
disabled and refreshed after the DAO enabled it must not remove a
co-delegator's contribution) and a mixed-percentage invariant check that the
delegate's total always equals the sum of scaled live delegations across
refreshes and expirations. All three refresh tests fail without the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBSNZfoCq2XqymbVV9r5rE
@noahsaso
noahsaso force-pushed the fix-delegation-refresh-weight branch from ba81189 to d4aee45 Compare September 1, 2026 20:56
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.11%. Comparing base (0178cf5) to head (d4aee45).

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #933      +/-   ##
===============================================
- Coverage        96.56%   92.11%   -4.46%     
===============================================
  Files              199      159      -40     
  Lines            67407    28775   -38632     
===============================================
- Hits             65094    26507   -38587     
+ Misses            2313     2268      -45     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants