fix reward distributor interval delay bug (#891)

DA0-DA0 · Oct 31, 2024 · ee38828 · ee38828
1 parent ce70d37
commit ee38828
Show file tree

Hide file tree

Showing 37 changed files with 534 additions and 331 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -18,7 +18,7 @@ resolver = "2"
 edition = "2021"
 license = "BSD-3-Clause"
 repository = "https://github.com/DA0-DA0/dao-contracts"
-version = "2.5.0"
+version = "2.5.1"
 
 [profile.release]
 codegen-units = 1
@@ -85,51 +85,51 @@ wynd-utils = "0.4"
 # optional owner.
 cw-ownable = "0.5"
 
-btsg-ft-factory = { path = "./contracts/external/btsg-ft-factory", version = "2.5.0" }
-cw-admin-factory = { path = "./contracts/external/cw-admin-factory", version = "2.5.0" }
-cw-denom = { path = "./packages/cw-denom", version = "2.5.0" }
-cw-fund-distributor = { path = "./contracts/distribution/cw-fund-distributor", version = "2.5.0" }
-cw-hooks = { path = "./packages/cw-hooks", version = "2.5.0" }
-cw-paginate-storage = { path = "./packages/cw-paginate-storage", version = "2.5.0" }
-cw-payroll-factory = { path = "./contracts/external/cw-payroll-factory", version = "2.5.0" }
-cw-stake-tracker = { path = "./packages/cw-stake-tracker", version = "2.5.0" }
-cw-token-swap = { path = "./contracts/external/cw-token-swap", version = "2.5.0" }
-cw-tokenfactory-issuer = { path = "./contracts/external/cw-tokenfactory-issuer", version = "2.5.0", default-features = false }
-cw-tokenfactory-types = { path = "./packages/cw-tokenfactory-types", version = "2.5.0", default-features = false }
-cw-vesting = { path = "./contracts/external/cw-vesting", version = "2.5.0" }
-cw-wormhole = { path = "./packages/cw-wormhole", version = "2.5.0" }
-cw20-stake = { path = "./contracts/staking/cw20-stake", version = "2.5.0" }
-cw20-stake-external-rewards = { path = "./contracts/staking/cw20-stake-external-rewards", version = "2.5.0" }
-cw20-stake-reward-distributor = { path = "./contracts/staking/cw20-stake-reward-distributor", version = "2.5.0" }
-cw721-controllers = { path = "./packages/cw721-controllers", version = "2.5.0" }
-cw721-roles = { path = "./contracts/external/cw721-roles", version = "2.5.0" }
-dao-cw721-extensions = { path = "./packages/dao-cw721-extensions", version = "2.5.0" }
-dao-dao-core = { path = "./contracts/dao-dao-core", version = "2.5.0" }
-dao-dao-macros = { path = "./packages/dao-dao-macros", version = "2.5.0" }
-dao-hooks = { path = "./packages/dao-hooks", version = "2.5.0" }
-dao-interface = { path = "./packages/dao-interface", version = "2.5.0" }
-dao-migrator = { path = "./contracts/external/dao-migrator", version = "2.5.0" }
-dao-pre-propose-approval-single = { path = "./contracts/pre-propose/dao-pre-propose-approval-single", version = "2.5.0" }
-dao-pre-propose-approver = { path = "./contracts/pre-propose/dao-pre-propose-approver", version = "2.5.0" }
-dao-pre-propose-base = { path = "./packages/dao-pre-propose-base", version = "2.5.0" }
-dao-pre-propose-multiple = { path = "./contracts/pre-propose/dao-pre-propose-multiple", version = "2.5.0" }
-dao-pre-propose-single = { path = "./contracts/pre-propose/dao-pre-propose-single", version = "2.5.0" }
-dao-proposal-condorcet = { path = "./contracts/proposal/dao-proposal-condorcet", version = "2.5.0" }
-dao-proposal-hook-counter = { path = "./contracts/test/dao-proposal-hook-counter", version = "2.5.0" }
-dao-proposal-multiple = { path = "./contracts/proposal/dao-proposal-multiple", version = "2.5.0" }
-dao-proposal-single = { path = "./contracts/proposal/dao-proposal-single", version = "2.5.0" }
-dao-proposal-sudo = { path = "./contracts/test/dao-proposal-sudo", version = "2.5.0" }
-dao-rewards-distributor = { path = "./contracts/distribution/dao-rewards-distributor", version = "2.5.0" }
-dao-test-custom-factory = { path = "./contracts/test/dao-test-custom-factory", version = "2.5.0" }
-dao-testing = { path = "./packages/dao-testing", version = "2.5.0" }
-dao-voting = { path = "./packages/dao-voting", version = "2.5.0" }
-dao-voting-cw20-balance = { path = "./contracts/test/dao-voting-cw20-balance", version = "2.5.0" }
-dao-voting-cw20-staked = { path = "./contracts/voting/dao-voting-cw20-staked", version = "2.5.0" }
-dao-voting-cw4 = { path = "./contracts/voting/dao-voting-cw4", version = "2.5.0" }
-dao-voting-cw721-roles = { path = "./contracts/voting/dao-voting-cw721-roles", version = "2.5.0" }
-dao-voting-cw721-staked = { path = "./contracts/voting/dao-voting-cw721-staked", version = "2.5.0" }
-dao-voting-onft-staked = { path = "./contracts/voting/dao-voting-onft-staked", version = "2.5.0" }
-dao-voting-token-staked = { path = "./contracts/voting/dao-voting-token-staked", version = "2.5.0" }
+btsg-ft-factory = { path = "./contracts/external/btsg-ft-factory", version = "2.5.1" }
+cw-admin-factory = { path = "./contracts/external/cw-admin-factory", version = "2.5.1" }
+cw-denom = { path = "./packages/cw-denom", version = "2.5.1" }
+cw-fund-distributor = { path = "./contracts/distribution/cw-fund-distributor", version = "2.5.1" }
+cw-hooks = { path = "./packages/cw-hooks", version = "2.5.1" }
+cw-paginate-storage = { path = "./packages/cw-paginate-storage", version = "2.5.1" }
+cw-payroll-factory = { path = "./contracts/external/cw-payroll-factory", version = "2.5.1" }
+cw-stake-tracker = { path = "./packages/cw-stake-tracker", version = "2.5.1" }
+cw-token-swap = { path = "./contracts/external/cw-token-swap", version = "2.5.1" }
+cw-tokenfactory-issuer = { path = "./contracts/external/cw-tokenfactory-issuer", version = "2.5.1", default-features = false }
+cw-tokenfactory-types = { path = "./packages/cw-tokenfactory-types", version = "2.5.1", default-features = false }
+cw-vesting = { path = "./contracts/external/cw-vesting", version = "2.5.1" }
+cw-wormhole = { path = "./packages/cw-wormhole", version = "2.5.1" }
+cw20-stake = { path = "./contracts/staking/cw20-stake", version = "2.5.1" }
+cw20-stake-external-rewards = { path = "./contracts/staking/cw20-stake-external-rewards", version = "2.5.1" }
+cw20-stake-reward-distributor = { path = "./contracts/staking/cw20-stake-reward-distributor", version = "2.5.1" }
+cw721-controllers = { path = "./packages/cw721-controllers", version = "2.5.1" }
+cw721-roles = { path = "./contracts/external/cw721-roles", version = "2.5.1" }
+dao-cw721-extensions = { path = "./packages/dao-cw721-extensions", version = "2.5.1" }
+dao-dao-core = { path = "./contracts/dao-dao-core", version = "2.5.1" }
+dao-dao-macros = { path = "./packages/dao-dao-macros", version = "2.5.1" }
+dao-hooks = { path = "./packages/dao-hooks", version = "2.5.1" }
+dao-interface = { path = "./packages/dao-interface", version = "2.5.1" }
+dao-migrator = { path = "./contracts/external/dao-migrator", version = "2.5.1" }
+dao-pre-propose-approval-single = { path = "./contracts/pre-propose/dao-pre-propose-approval-single", version = "2.5.1" }
+dao-pre-propose-approver = { path = "./contracts/pre-propose/dao-pre-propose-approver", version = "2.5.1" }
+dao-pre-propose-base = { path = "./packages/dao-pre-propose-base", version = "2.5.1" }
+dao-pre-propose-multiple = { path = "./contracts/pre-propose/dao-pre-propose-multiple", version = "2.5.1" }
+dao-pre-propose-single = { path = "./contracts/pre-propose/dao-pre-propose-single", version = "2.5.1" }
+dao-proposal-condorcet = { path = "./contracts/proposal/dao-proposal-condorcet", version = "2.5.1" }
+dao-proposal-hook-counter = { path = "./contracts/test/dao-proposal-hook-counter", version = "2.5.1" }
+dao-proposal-multiple = { path = "./contracts/proposal/dao-proposal-multiple", version = "2.5.1" }
+dao-proposal-single = { path = "./contracts/proposal/dao-proposal-single", version = "2.5.1" }
+dao-proposal-sudo = { path = "./contracts/test/dao-proposal-sudo", version = "2.5.1" }
+dao-rewards-distributor = { path = "./contracts/distribution/dao-rewards-distributor", version = "2.5.1" }
+dao-test-custom-factory = { path = "./contracts/test/dao-test-custom-factory", version = "2.5.1" }
+dao-testing = { path = "./packages/dao-testing", version = "2.5.1" }
+dao-voting = { path = "./packages/dao-voting", version = "2.5.1" }
+dao-voting-cw20-balance = { path = "./contracts/test/dao-voting-cw20-balance", version = "2.5.1" }
+dao-voting-cw20-staked = { path = "./contracts/voting/dao-voting-cw20-staked", version = "2.5.1" }
+dao-voting-cw4 = { path = "./contracts/voting/dao-voting-cw4", version = "2.5.1" }
+dao-voting-cw721-roles = { path = "./contracts/voting/dao-voting-cw721-roles", version = "2.5.1" }
+dao-voting-cw721-staked = { path = "./contracts/voting/dao-voting-cw721-staked", version = "2.5.1" }
+dao-voting-onft-staked = { path = "./contracts/voting/dao-voting-onft-staked", version = "2.5.1" }
+dao-voting-token-staked = { path = "./contracts/voting/dao-voting-token-staked", version = "2.5.1" }
 
 # v1 dependencies. used for state migrations.
 cw-core-v1 = { package = "cw-core", version = "0.1.0" }

diff --git a/contracts/dao-dao-core/schema/dao-dao-core.json b/contracts/dao-dao-core/schema/dao-dao-core.json
@@ -1,6 +1,6 @@
 {
   "contract_name": "dao-dao-core",
-  "contract_version": "2.5.0",
+  "contract_version": "2.5.1",
   "idl_version": "1.0.0",
   "instantiate": {
     "$schema": "http://json-schema.org/draft-07/schema#",

diff --git a/contracts/distribution/cw-fund-distributor/schema/cw-fund-distributor.json b/contracts/distribution/cw-fund-distributor/schema/cw-fund-distributor.json
@@ -1,6 +1,6 @@
 {
   "contract_name": "cw-fund-distributor",
-  "contract_version": "2.5.0",
+  "contract_version": "2.5.1",
   "idl_version": "1.0.0",
   "instantiate": {
     "$schema": "http://json-schema.org/draft-07/schema#",

diff --git a/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json b/contracts/distribution/dao-rewards-distributor/schema/dao-rewards-distributor.json
@@ -1,6 +1,6 @@
 {
   "contract_name": "dao-rewards-distributor",
-  "contract_version": "2.5.0",
+  "contract_version": "2.5.1",
   "idl_version": "1.0.0",
   "instantiate": {
     "$schema": "http://json-schema.org/draft-07/schema#",
@@ -225,6 +225,28 @@
         },
         "additionalProperties": false
       },
+      {
+        "description": "forcibly withdraw funds from the contract. this is unsafe and should only be used to recover funds that are stuck in the contract.",
+        "type": "object",
+        "required": [
+          "unsafe_force_withdraw"
+        ],
+        "properties": {
+          "unsafe_force_withdraw": {
+            "type": "object",
+            "required": [
+              "amount"
+            ],
+            "properties": {
+              "amount": {
+                "$ref": "#/definitions/Coin"
+              }
+            },
+            "additionalProperties": false
+          }
+        },
+        "additionalProperties": false
+      },
       {
         "description": "Update the contract's ownership. The `action` to be provided can be either to propose transferring ownership to an account, accept a pending ownership transfer, or renounce the ownership permanently.",
         "type": "object",
@@ -299,6 +321,21 @@
         "description": "Binary is a wrapper around Vec<u8> to add base64 de/serialization with serde. It also adds some helper methods to help encode inline.\n\nThis is only needed as serde-json-{core,wasm} has a horrible encoding for Vec<u8>. See also <https://github.com/CosmWasm/cosmwasm/blob/main/docs/MESSAGE_TYPES.md>.",
         "type": "string"
       },
+      "Coin": {
+        "type": "object",
+        "required": [
+          "amount",
+          "denom"
+        ],
+        "properties": {
+          "amount": {
+            "$ref": "#/definitions/Uint128"
+          },
+          "denom": {
+            "type": "string"
+          }
+        }
+      },
       "CreateMsg": {
         "type": "object",
         "required": [

diff --git a/contracts/distribution/dao-rewards-distributor/src/contract.rs b/contracts/distribution/dao-rewards-distributor/src/contract.rs
@@ -1,8 +1,8 @@
 #[cfg(not(feature = "library"))]
 use cosmwasm_std::entry_point;
 use cosmwasm_std::{
-    ensure, from_json, to_json_binary, Addr, Binary, Deps, DepsMut, Env, MessageInfo, Order,
-    Response, StdError, StdResult, Uint128, Uint256,
+    ensure, from_json, to_json_binary, Addr, BankMsg, Binary, Coin, CosmosMsg, Deps, DepsMut, Env,
+    MessageInfo, Order, Response, StdError, StdResult, Uint128, Uint256,
 };
 use cw2::{get_contract_version, set_contract_version};
 use cw20::{Cw20ReceiveMsg, Denom};
@@ -91,6 +91,9 @@ pub fn execute(
         ExecuteMsg::FundLatest {} => execute_fund_latest_native(deps, env, info),
         ExecuteMsg::Claim { id } => execute_claim(deps, env, info, id),
         ExecuteMsg::Withdraw { id } => execute_withdraw(deps, info, env, id),
+        ExecuteMsg::UnsafeForceWithdraw { amount } => {
+            execute_unsafe_force_withdraw(deps, info, amount)
+        }
     }
 }
 
@@ -594,6 +597,27 @@ fn execute_update_owner(
     Ok(Response::new().add_attributes(ownership.into_attributes()))
 }
 
+fn execute_unsafe_force_withdraw(
+    deps: DepsMut,
+    info: MessageInfo,
+    amount: Coin,
+) -> Result<Response, ContractError> {
+    nonpayable(&info)?;
+
+    // only the owner can initiate a force withdraw
+    cw_ownable::assert_owner(deps.storage, &info.sender)?;
+
+    let send = CosmosMsg::Bank(BankMsg::Send {
+        to_address: info.sender.to_string(),
+        amount: vec![amount.clone()],
+    });
+
+    Ok(Response::new()
+        .add_message(send)
+        .add_attribute("action", "unsafe_force_withdraw")
+        .add_attribute("amount", amount.to_string()))
+}
+
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn query(deps: Deps, env: Env, msg: QueryMsg) -> StdResult<Binary> {
     match msg {
@@ -737,7 +761,7 @@ pub fn migrate(deps: DepsMut, _env: Env, _msg: MigrateMsg) -> Result<Response, C
 
     // only allow upgrades
     if new_version <= current_version {
-        return Err(ContractError::MigrationErrorInvalidVersion {
+        return Err(ContractError::MigrationErrorInvalidVersionNotNewer {
             new: new_version.to_string(),
             current: current_version.to_string(),
         });

diff --git a/contracts/distribution/dao-rewards-distributor/src/error.rs b/contracts/distribution/dao-rewards-distributor/src/error.rs
@@ -1,4 +1,6 @@
-use cosmwasm_std::{DivideByZeroError, OverflowError, StdError};
+use cosmwasm_std::{
+    CheckedFromRatioError, CheckedMultiplyFractionError, DivideByZeroError, OverflowError, StdError,
+};
 use cw_utils::PaymentError;
 use thiserror::Error;
 
@@ -19,6 +21,12 @@ pub enum ContractError {
     #[error(transparent)]
     DivideByZero(#[from] DivideByZeroError),
 
+    #[error(transparent)]
+    CheckedFromRatio(#[from] CheckedFromRatioError),
+
+    #[error(transparent)]
+    CheckedMultiplyFraction(#[from] CheckedMultiplyFractionError),
+
     #[error(transparent)]
     Payment(#[from] PaymentError),
 
@@ -59,7 +67,7 @@ pub enum ContractError {
     DistributionHistoryTooLarge { err: String },
 
     #[error("Invalid version migration. {new} is not newer than {current}.")]
-    MigrationErrorInvalidVersion { new: String, current: String },
+    MigrationErrorInvalidVersionNotNewer { new: String, current: String },
 
     #[error("Expected to migrate from contract {expected}. Got {actual}.")]
     MigrationErrorIncorrectContract { expected: String, actual: String },

diff --git a/contracts/distribution/dao-rewards-distributor/src/helpers.rs b/contracts/distribution/dao-rewards-distributor/src/helpers.rs
@@ -1,6 +1,6 @@
 use cosmwasm_std::{
-    coins, to_json_binary, Addr, BankMsg, BlockInfo, CosmosMsg, Deps, DepsMut, StdError, StdResult,
-    Uint128, Uint256, WasmMsg,
+    coins, to_json_binary, Addr, BankMsg, BlockInfo, CosmosMsg, Decimal, Deps, DepsMut, StdError,
+    StdResult, Uint128, Uint256, WasmMsg,
 };
 use cw20::{Denom, Expiration};
 use cw_utils::Duration;
@@ -117,9 +117,9 @@ pub trait DurationExt {
     /// Returns true if the duration is 0 blocks or 0 seconds.
     fn is_zero(&self) -> bool;
 
-    /// Perform checked integer division between two durations, erroring if the
-    /// units do not match or denominator is 0.
-    fn checked_div(&self, denominator: &Self) -> Result<Uint128, ContractError>;
+    /// Returns the ratio between the two durations (numerator / denominator) as
+    /// a Decimal, erroring if the units do not match.
+    fn ratio(&self, denominator: &Self) -> Result<Decimal, ContractError>;
 }
 
 impl DurationExt for Duration {
@@ -130,13 +130,13 @@ impl DurationExt for Duration {
         }
     }
 
-    fn checked_div(&self, denominator: &Self) -> Result<Uint128, ContractError> {
+    fn ratio(&self, denominator: &Self) -> Result<Decimal, ContractError> {
         match (self, denominator) {
             (Duration::Height(numerator), Duration::Height(denominator)) => {
-                Ok(Uint128::from(*numerator).checked_div(Uint128::from(*denominator))?)
+                Ok(Decimal::checked_from_ratio(*numerator, *denominator)?)
             }
             (Duration::Time(numerator), Duration::Time(denominator)) => {
-                Ok(Uint128::from(*numerator).checked_div(Uint128::from(*denominator))?)
+                Ok(Decimal::checked_from_ratio(*numerator, *denominator)?)
             }
             _ => Err(ContractError::Std(StdError::generic_err(format!(
                 "incompatible durations: got numerator {:?} and denominator {:?}",

diff --git a/contracts/distribution/dao-rewards-distributor/src/msg.rs b/contracts/distribution/dao-rewards-distributor/src/msg.rs
@@ -1,5 +1,5 @@
 use cosmwasm_schema::{cw_serde, QueryResponses};
-use cosmwasm_std::Uint128;
+use cosmwasm_std::{Coin, Uint128};
 use cw20::{Cw20ReceiveMsg, Denom, UncheckedDenom};
 use cw4::MemberChangedHookMsg;
 use cw_ownable::cw_ownable_execute;
@@ -60,6 +60,9 @@ pub enum ExecuteMsg {
     /// claim whatever they earned until this point. this is effectively an
     /// inverse to fund and does not affect any already-distributed rewards.
     Withdraw { id: u64 },
+    /// forcibly withdraw funds from the contract. this is unsafe and should
+    /// only be used to recover funds that are stuck in the contract.
+    UnsafeForceWithdraw { amount: Coin },
 }
 
 #[cw_serde]

diff --git a/contracts/distribution/dao-rewards-distributor/src/rewards.rs b/contracts/distribution/dao-rewards-distributor/src/rewards.rs
@@ -121,17 +121,13 @@ pub fn get_active_total_earned_puvp(
             if total_power.is_zero() {
                 Ok(curr)
             } else {
-                // count intervals of the rewards emission that have passed
-                // since the last update which need to be distributed
+                // count (partial) intervals of the rewards emission that have
+                // passed since the last update which need to be distributed
                 let complete_distribution_periods =
-                    new_reward_distribution_duration.checked_div(&duration)?;
-
-                // It is impossible for this to overflow as total rewards can
-                // never exceed max value of Uint128 as total tokens in
-                // existence cannot exceed Uint128 (because the bank module Coin
-                // type uses Uint128).
-                let new_rewards_distributed = amount
-                    .full_mul(complete_distribution_periods)
+                    new_reward_distribution_duration.ratio(&duration)?;
+
+                let new_rewards_distributed = Uint256::from(amount)
+                    .checked_mul_floor(complete_distribution_periods)?
                     .checked_mul(scale_factor())?;
 
                 // the new rewards per unit voting power that have been

diff --git a/contracts/distribution/dao-rewards-distributor/src/state.rs b/contracts/distribution/dao-rewards-distributor/src/state.rs
@@ -246,9 +246,9 @@ impl DistributionState {
 
                 // count total intervals of the rewards emission that will pass
                 // based on the start and end times.
-                let complete_distribution_periods = epoch_duration.checked_div(&duration)?;
+                let complete_distribution_periods = epoch_duration.ratio(&duration)?;
 
-                Ok(amount.checked_mul(complete_distribution_periods)?)
+                Ok(amount.checked_mul_floor(complete_distribution_periods)?)
             }
         }
     }

diff --git a/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs b/contracts/distribution/dao-rewards-distributor/src/testing/suite.rs
@@ -974,4 +974,32 @@ impl Suite {
             )
             .unwrap();
     }
+
+    pub fn unsafe_force_withdraw(&mut self, amount: Coin) {
+        let msg = ExecuteMsg::UnsafeForceWithdraw { amount };
+        self.base
+            .app
+            .execute_contract(
+                Addr::unchecked(OWNER),
+                self.distribution_contract.clone(),
+                &msg,
+                &[],
+            )
+            .unwrap();
+    }
+
+    pub fn unsafe_force_withdraw_unauthorized(&mut self, amount: Coin) -> ContractError {
+        let msg = ExecuteMsg::UnsafeForceWithdraw { amount };
+        self.base
+            .app
+            .execute_contract(
+                Addr::unchecked("no_one"),
+                self.distribution_contract.clone(),
+                &msg,
+                &[],
+            )
+            .unwrap_err()
+            .downcast()
+            .unwrap()
+    }
 }