From a076844a600068d176bfa9d19689f6648cb8af78 Mon Sep 17 00:00:00 2001 From: tauagent <259104564+tauagent@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:05:56 +0000 Subject: [PATCH 1/2] Fix delegated voting power refresh accounting --- .../dao-vote-delegation/src/helpers.rs | 27 +++++++------- .../dao-vote-delegation/src/testing/tests.rs | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/contracts/delegation/dao-vote-delegation/src/helpers.rs b/contracts/delegation/dao-vote-delegation/src/helpers.rs index eef141cf6..746841787 100644 --- a/contracts/delegation/dao-vote-delegation/src/helpers.rs +++ b/contracts/delegation/dao-vote-delegation/src/helpers.rs @@ -89,7 +89,7 @@ pub fn add_delegated_vp( storage: &mut dyn Storage, env: &Env, delegate: &Addr, - vp: Uint128, + delegated_vp: Uint128, expiration: Option, ) -> StdResult<()> { DELEGATED_VP.increment( @@ -103,12 +103,12 @@ pub fn add_delegated_vp( // update the total that will be reflected in historical queries // starting from the next block. env.block.height + 1, - vp, + delegated_vp, )?; // if expiration exists, decrement in the future at expiration height if let Some(expiration) = expiration { - DELEGATED_VP.decrement(storage, delegate.clone(), expiration, vp)?; + DELEGATED_VP.decrement(storage, delegate.clone(), expiration, delegated_vp)?; } Ok(()) @@ -122,7 +122,7 @@ pub fn remove_delegated_vp_if_not_expired( storage: &mut dyn Storage, env: &Env, delegate: &Addr, - vp: Uint128, + delegated_vp: Uint128, original_expiration: Option, ) -> StdResult<()> { // if delegation already expired, do nothing. @@ -136,7 +136,7 @@ pub fn remove_delegated_vp_if_not_expired( // decrement at end of expiration period. do this before undoing previous // increment to prevent underflow. if let Some(original_expiration) = original_expiration { - DELEGATED_VP.increment(storage, delegate.clone(), original_expiration, vp)?; + DELEGATED_VP.increment(storage, delegate.clone(), original_expiration, delegated_vp)?; } DELEGATED_VP.decrement( @@ -150,7 +150,7 @@ pub fn remove_delegated_vp_if_not_expired( // update the total that will be reflected in historical queries // starting from the next block. env.block.height + 1, - vp, + delegated_vp, )?; Ok(()) @@ -161,7 +161,7 @@ pub fn update_delegated_vp_expiration( storage: &mut dyn Storage, env: &Env, delegate: &Addr, - vp: Uint128, + delegated_vp: Uint128, original_expiration: Option, new_expiration: Option, ) -> StdResult<()> { @@ -174,7 +174,7 @@ pub fn update_delegated_vp_expiration( )); } - DELEGATED_VP.increment(storage, delegate.clone(), original_expiration, vp)?; + DELEGATED_VP.increment(storage, delegate.clone(), original_expiration, delegated_vp)?; } // if new expiration is set, decrement at new expiration @@ -185,7 +185,7 @@ pub fn update_delegated_vp_expiration( )); } - DELEGATED_VP.decrement(storage, delegate.clone(), new_expiration, vp)?; + DELEGATED_VP.decrement(storage, delegate.clone(), new_expiration, delegated_vp)?; } Ok(()) @@ -239,7 +239,7 @@ pub fn handle_redelegation( new_percent: Decimal, config: &Config, current_percent_delegated: Decimal, - vp: Uint128, + delegator_vp: Uint128, existing_delegation_entry: SnapshotVectorMapItemRef, ) -> DelegationHandlerResult { let (existing_delegation_id, existing_delegation_expiration) = existing_delegation_entry; @@ -269,11 +269,12 @@ pub fn handle_redelegation( config.delegation_validity_blocks, )?; + let delegated_vp = calculate_delegated_vp(delegator_vp, new_percent); update_delegated_vp_expiration( deps.storage, env, delegate, - vp, + delegated_vp, existing_delegation_expiration, new_expiration, )?; @@ -294,12 +295,12 @@ pub fn handle_redelegation( if !expired { // remove current delegated VP based on existing percent - let old_vp = calculate_delegated_vp(vp, existing_delegation_percent); + let old_delegated_vp = calculate_delegated_vp(delegator_vp, existing_delegation_percent); remove_delegated_vp_if_not_expired( deps.storage, env, delegate, - old_vp, + old_delegated_vp, existing_delegation_expiration, )?; } diff --git a/contracts/delegation/dao-vote-delegation/src/testing/tests.rs b/contracts/delegation/dao-vote-delegation/src/testing/tests.rs index 0cb03ed8a..c0db8c235 100644 --- a/contracts/delegation/dao-vote-delegation/src/testing/tests.rs +++ b/contracts/delegation/dao-vote-delegation/src/testing/tests.rs @@ -743,6 +743,42 @@ fn test_expiration_update() { suite.assert_delegate_total_delegated_vp(ADDR0, suite.members[1].weight); } +#[test] +fn test_same_percent_refresh_moves_only_delegated_voting_power() { + let mut suite = Cw4DaoVoteDelegationTestingSuite::new() + .with_delegation_validity_blocks(10) + .build(); + + suite.register(ADDR0); + + let refreshed_vp = Uint128::from(suite.members[1].weight).mul_floor(Decimal::percent(50)); + let co_delegator_vp = Uint128::from(suite.members[2].weight).mul_floor(Decimal::percent(50)); + assert!(!co_delegator_vp.is_zero()); + + suite.delegate(ADDR1, ADDR0, Decimal::percent(50)); + suite.delegate(ADDR2, ADDR0, Decimal::percent(50)); + suite.advance_block(); + + suite.assert_delegate_total_delegated_vp(ADDR0, refreshed_vp + co_delegator_vp); + + // Refresh ADDR1 halfway through the validity period without changing its + // percentage. ADDR2 retains the original expiration. + suite.advance_blocks(4); + suite.delegate(ADDR1, ADDR0, Decimal::percent(50)); + + // Refreshing only moves ADDR1's scaled contribution to the new expiration. + suite.assert_delegate_total_delegated_vp(ADDR0, refreshed_vp + co_delegator_vp); + + // At the original expiration, ADDR2's contribution expires while ADDR1's + // refreshed, scaled contribution remains. + suite.advance_blocks(5); + suite.assert_delegate_total_delegated_vp(ADDR0, refreshed_vp); + + // The refreshed contribution expires at its new expiration. + suite.advance_blocks(5); + suite.assert_delegate_total_delegated_vp(ADDR0, 0u128); +} + #[test] fn test_max_delegations() { let mut suite = Cw4DaoVoteDelegationTestingSuite::new() From d4aee45855d1a87e181e2a2afcece2758d8a168b Mon Sep 17 00:00:00 2001 From: Noah Saso <6721426+NoahSaso@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:33:56 -0400 Subject: [PATCH 2/2] Add regression tests for delegation refresh accounting 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 Claude-Session: https://claude.ai/code/session_01YBSNZfoCq2XqymbVV9r5rE --- .../dao-vote-delegation/src/testing/tests.rs | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/contracts/delegation/dao-vote-delegation/src/testing/tests.rs b/contracts/delegation/dao-vote-delegation/src/testing/tests.rs index c0db8c235..c7c8ca602 100644 --- a/contracts/delegation/dao-vote-delegation/src/testing/tests.rs +++ b/contracts/delegation/dao-vote-delegation/src/testing/tests.rs @@ -779,6 +779,102 @@ fn test_same_percent_refresh_moves_only_delegated_voting_power() { suite.assert_delegate_total_delegated_vp(ADDR0, 0u128); } +#[test] +fn test_same_percent_refresh_after_enabling_expiration_preserves_co_delegator_vp() { + // start with delegation expiration disabled. + let mut suite = Cw4DaoVoteDelegationTestingSuite::new().build(); + + suite.register(ADDR0); + + let addr1_delegated_vp = Uint128::from(suite.members[1].weight).mul_floor(Decimal::percent(50)); + let addr2_delegated_vp = Uint128::from(suite.members[2].weight).mul_floor(Decimal::percent(50)); + assert!(!addr1_delegated_vp.is_zero()); + assert!(!addr2_delegated_vp.is_zero()); + + suite.delegate(ADDR1, ADDR0, Decimal::percent(50)); + suite.delegate(ADDR2, ADDR0, Decimal::percent(50)); + suite.advance_block(); + + suite.assert_delegate_total_delegated_vp(ADDR0, addr1_delegated_vp + addr2_delegated_vp); + + // enable expiration. existing delegations keep no expiration until they + // are refreshed. + suite.update_delegation_validity_blocks(Some(10)); + + // ADDR1 refreshes with the same percent, picking up the new expiration. + // there is no original expiration to undo, so the only scheduled change + // must be the removal of ADDR1's scaled contribution at the new + // expiration. + suite.delegate(ADDR1, ADDR0, Decimal::percent(50)); + let expiration = suite.app.block_info().height + 10; + suite.assert_delegation(ADDR1, ADDR0, Decimal::percent(50), Some(expiration)); + suite.assert_delegation(ADDR2, ADDR0, Decimal::percent(50), None); + suite.assert_delegate_total_delegated_vp(ADDR0, addr1_delegated_vp + addr2_delegated_vp); + + // once ADDR1's refreshed delegation expires, only ADDR1's scaled + // contribution is removed. ADDR2's delegation never expires and must be + // untouched. + suite.advance_blocks(10); + suite.assert_delegations_count(ADDR1, 0); + suite.assert_delegations_count(ADDR2, 1); + suite.assert_delegate_total_delegated_vp(ADDR0, addr2_delegated_vp); +} + +#[test] +fn test_mixed_percent_refreshes_keep_total_equal_to_sum_of_scaled_delegations() { + let mut suite = Cw4DaoVoteDelegationTestingSuite::new() + .with_delegation_validity_blocks(10) + .build(); + + suite.register(ADDR0); + + // delegators with different weights and percents, none of which delegate + // their full weight except the last, so that raw and scaled amounts differ. + let delegations = [ + (ADDR1, suite.members[1].weight, Decimal::percent(60)), + (ADDR2, suite.members[2].weight, Decimal::percent(50)), + (ADDR3, suite.members[3].weight, Decimal::percent(75)), + (ADDR4, suite.members[4].weight, Decimal::percent(100)), + ]; + let scaled = |i: usize| Uint128::from(delegations[i].1).mul_floor(delegations[i].2); + let raw_total: u64 = delegations.iter().map(|(_, weight, _)| weight).sum(); + let scaled_total = (0..delegations.len()).map(scaled).sum::(); + assert!(scaled_total < Uint128::from(raw_total)); + assert!((0..delegations.len()).all(|i| !scaled(i).is_zero())); + + for (delegator, _, percent) in delegations { + suite.delegate(delegator, ADDR0, percent); + } + let original_expiration = suite.app.block_info().height + 10; + suite.advance_block(); + + suite.assert_delegate_total_delegated_vp(ADDR0, scaled_total); + + // refresh two of the delegations partway through the validity period. + suite.advance_blocks(3); + suite.delegate(ADDR1, ADDR0, delegations[0].2); + suite.delegate(ADDR3, ADDR0, delegations[2].2); + let refreshed_expiration = suite.app.block_info().height + 10; + assert!(refreshed_expiration > original_expiration); + + suite.assert_delegate_total_delegated_vp(ADDR0, scaled_total); + + // at the original expiration, only the two delegations that were not + // refreshed expire. + let blocks_until_original_expiration = original_expiration - suite.app.block_info().height; + suite.advance_blocks(blocks_until_original_expiration); + suite.assert_delegations_count(ADDR2, 0); + suite.assert_delegations_count(ADDR4, 0); + suite.assert_delegate_total_delegated_vp(ADDR0, scaled(0) + scaled(2)); + + // at the refreshed expiration, the remaining delegations expire. + let blocks_until_refreshed_expiration = refreshed_expiration - suite.app.block_info().height; + suite.advance_blocks(blocks_until_refreshed_expiration); + suite.assert_delegations_count(ADDR1, 0); + suite.assert_delegations_count(ADDR3, 0); + suite.assert_delegate_total_delegated_vp(ADDR0, 0u128); +} + #[test] fn test_max_delegations() { let mut suite = Cw4DaoVoteDelegationTestingSuite::new()