feat(incentives): enforce max gas limit on token transfers (#434)

* feat(incentives): enforce max gas limit on token transfers

* bump astroport version

* convince clippy

Cargo.toml

@@ -25,7 +25,7 @@ thiserror = "1.0"
 itertools = "0.12"
 cosmwasm-schema = "1.5"
 cw-utils = "1"
-astroport = { path = "./packages/astroport", version = "5.6.0" }
+astroport = { path = "./packages/astroport", version = "5.7.0" }
 
 [profile.release]
 opt-level = "z"

contracts/tokenomics/incentives/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "astroport-incentives"
-version = "1.2.0"
+version = "1.3.0"
 authors = ["Astroport"]
 edition = "2021"
 description = "Astroport Incentives Contract distributing rewards to LP stakers"

contracts/tokenomics/incentives/src/execute.rs

@@ -15,7 +15,9 @@ use astroport::asset::{
 use astroport::common::{claim_ownership, drop_ownership_proposal, propose_new_owner};
 use astroport::factory;
 use astroport::factory::PairType;
-use astroport::incentives::{Cw20Msg, ExecuteMsg, IncentivizationFeeInfo, RewardType};
+use astroport::incentives::{
+    Cw20Msg, ExecuteMsg, IncentivizationFeeInfo, RewardType, TOKEN_TRANSFER_GAS_LIMIT,
+};
 
 use crate::error::ContractError;
 use crate::state::{
@@ -61,7 +63,8 @@ pub fn execute(
                 .collect_vec();
 
             // Compose response. Return early in case of error
-            let response = claim_rewards(deps.storage, None, env, &info.sender, mut_tuples)?;
+            let config = CONFIG.load(deps.storage)?;
+            let response = claim_rewards(deps.storage, &config, env, &info.sender, mut_tuples)?;
 
             // Save updates in state
             for (lp_asset, pool_info, user_pos) in tuples {
@@ -121,6 +124,7 @@ pub fn execute(
             generator_controller,
             guardian,
             incentivization_fee_info,
+            token_transfer_gas_limit,
         } => update_config(
             deps,
             info,
@@ -129,6 +133,7 @@ pub fn execute(
             generator_controller,
             guardian,
             incentivization_fee_info,
+            token_transfer_gas_limit,
         ),
         ExecuteMsg::UpdateBlockedTokenslist { add, remove } => {
             update_blocked_pool_tokens(deps, env, info, add, remove)
@@ -193,7 +198,7 @@ fn deposit(
 
     let response = claim_rewards(
         deps.storage,
-        Some(config.vesting_contract),
+        &config,
         env,
         &staker,
         vec![(&maybe_lp.info, &mut pool_info, &mut user_info)],
@@ -230,9 +235,10 @@ fn withdraw(
     } else {
         let mut pool_info = PoolInfo::load(deps.storage, &lp_token_asset)?;
 
+        let config = CONFIG.load(deps.storage)?;
         let response = claim_rewards(
             deps.storage,
-            None,
+            &config,
             env,
             &info.sender,
             vec![(&lp_token_asset, &mut pool_info, &mut user_info)],
@@ -372,6 +378,7 @@ fn set_tokens_per_second(
     Ok(Response::new().add_attribute("action", "set_tokens_per_second"))
 }
 
+#[allow(clippy::too_many_arguments)]
 fn update_config(
     deps: DepsMut,
     info: MessageInfo,
@@ -380,6 +387,7 @@ fn update_config(
     generator_controller: Option<String>,
     guardian: Option<String>,
     incentivization_fee_info: Option<IncentivizationFeeInfo>,
+    token_transfer_gas_limit: Option<u64>,
 ) -> Result<Response, ContractError> {
     let mut config = CONFIG.load(deps.storage)?;
 
@@ -439,6 +447,19 @@ fn update_config(
         config.incentivization_fee_info = Some(new_info);
     }
 
+    if let Some(token_transfer_gas_limit) = token_transfer_gas_limit {
+        ensure!(
+            TOKEN_TRANSFER_GAS_LIMIT.contains(&token_transfer_gas_limit),
+            StdError::generic_err("Invalid token transfer gas limit")
+        );
+
+        attrs.push(attr(
+            "new_token_transfer_gas_limit",
+            token_transfer_gas_limit.to_string(),
+        ));
+        config.token_transfer_gas_limit = Some(token_transfer_gas_limit);
+    }
+
     CONFIG.save(deps.storage, &config)?;
 
     Ok(Response::new().add_attributes(attrs))

contracts/tokenomics/incentives/src/instantiate.rs

@@ -41,6 +41,7 @@ pub fn instantiate(
             vesting_contract: deps.api.addr_validate(&msg.vesting_contract)?,
             guardian: addr_opt_validate(deps.api, &msg.guardian)?,
             incentivization_fee_info: msg.incentivization_fee_info,
+            token_transfer_gas_limit: None,
         },
     )?;
     ACTIVE_POOLS.save(deps.storage, &vec![])?;

contracts/tokenomics/incentives/src/migrate.rs

@@ -13,7 +13,7 @@ pub fn migrate(deps: DepsMut, _env: Env, _msg: Empty) -> Result<Response, Contra
 
     match contract_version.contract.as_ref() {
         "astroport-incentives" => match contract_version.version.as_ref() {
-            "1.0.0" | "1.0.1" | "1.1.0" => {}
+            "1.0.0" | "1.0.1" | "1.1.0" | "1.2.0" => {}
             _ => return Err(ContractError::MigrationError {}),
         },
         _ => return Err(ContractError::MigrationError {}),

contracts/tokenomics/incentives/src/reply.rs

@@ -10,9 +10,12 @@ pub const POST_TRANSFER_REPLY_ID: u64 = 1;
 #[cfg_attr(not(feature = "library"), entry_point)]
 pub fn reply(_deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractError> {
     match msg {
-        // Caller context: either utils:claim_rewards() or utils:remove_reward_from_pool().
-        // If cw20 token reverts the transfer, we bypass it silently.
-        // This can happen in abnormal situations when cw20 contract was tweaked and broken.
+        // Caller context: either utils:claim_rewards(),
+        // utils:claim_orphaned_rewards() or utils:remove_reward_from_pool().
+        // If cw20 or token factory token with bank hook reverts the transfer,
+        // we bypass it silently.
+        // This error also can be reached if token transfer hits gas limit
+        // (see astroport/incentives.rs:Config:token_transfer_gas_limit).
         Reply {
             id: POST_TRANSFER_REPLY_ID,
             result: SubMsgResult::Err(err_msg),

contracts/tokenomics/incentives/src/utils.rs

@@ -23,7 +23,7 @@ use crate::state::{
 /// If vesting_contract is None this function reads config from state and gets vesting address.
 pub fn claim_rewards(
     storage: &dyn Storage,
-    vesting_contract: Option<Addr>,
+    config: &Config,
     env: Env,
     user: &Addr,
     pool_tuples: Vec<(&AssetInfo, &mut PoolInfo, &mut UserInfo)>,
@@ -71,20 +71,18 @@ pub fn claim_rewards(
         .into_iter()
         .map(|(info, assets)| {
             let amount: Uint128 = assets.into_iter().map(|asset| asset.amount).sum();
-            info.with_balance(amount)
-                .into_submsg(user, Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)))
+            info.with_balance(amount).into_submsg(
+                user,
+                Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)),
+                config.token_transfer_gas_limit,
+            )
         })
         .collect::<StdResult<Vec<_>>>()?;
 
     // Claim Astroport rewards
     if !protocol_reward_amount.is_zero() {
-        let vesting_contract = if let Some(vesting_contract) = vesting_contract {
-            vesting_contract
-        } else {
-            CONFIG.load(storage)?.vesting_contract
-        };
         messages.push(SubMsg::new(wasm_execute(
-            vesting_contract,
+            &config.vesting_contract,
             &vesting::ExecuteMsg::Claim {
                 recipient: Some(user.to_string()),
                 amount: Some(protocol_reward_amount),
@@ -366,9 +364,11 @@ pub fn remove_reward_from_pool(
     // Send unclaimed rewards
     if !unclaimed.is_zero() {
         deps.api.addr_validate(&receiver)?;
-        let transfer_msg = reward_asset
-            .with_balance(unclaimed)
-            .into_submsg(receiver, Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)))?;
+        let transfer_msg = reward_asset.with_balance(unclaimed).into_submsg(
+            receiver,
+            Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)),
+            config.token_transfer_gas_limit,
+        )?;
         response = response.add_submessage(transfer_msg);
     }
 
@@ -423,8 +423,7 @@ pub fn is_pool_registered(
             ))
         })
         .map(|resp| {
-            // Eventually resp.liquidity_token will become just a String once token factory LP tokens are implemented
-            if resp.liquidity_token.as_str() == lp_token_addr {
+            if resp.liquidity_token == lp_token_addr {
                 Ok(())
             } else {
                 Err(StdError::generic_err(format!(
@@ -474,8 +473,11 @@ pub fn claim_orphaned_rewards(
 
             attrs.push(attr("claimed_orphaned_reward", reward_asset.to_string()));
 
-            let transfer_msg = reward_asset
-                .into_submsg(&receiver, Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)))?;
+            let transfer_msg = reward_asset.into_submsg(
+                &receiver,
+                Some((ReplyOn::Error, POST_TRANSFER_REPLY_ID)),
+                config.token_transfer_gas_limit,
+            )?;
             messages.push(transfer_msg);
         }
     }

contracts/tokenomics/incentives/tests/incentives_integration_tests.rs

@@ -1294,6 +1294,7 @@ fn test_astro_protocol_reward_if_denom_changed() {
         generator_controller: None,
         guardian: None,
         incentivization_fee_info: None,
+        token_transfer_gas_limit: None,
     };
     helper
         .app
@@ -2055,13 +2056,15 @@ fn test_update_config() {
         fee_receiver: TestAddr::new("new_fee_receiver"),
         fee: coin(1000, "uusd"),
     };
+    let new_gas_limit = 800_000;
 
     let msg = ExecuteMsg::UpdateConfig {
         astro_token: Some(AssetInfo::native("new_astro")),
         vesting_contract: Some(new_vesting.to_string()),
         generator_controller: Some(new_generator_controller.to_string()),
         guardian: Some(new_guardian.to_string()),
         incentivization_fee_info: Some(new_incentivization_fee_info.clone()),
+        token_transfer_gas_limit: Some(new_gas_limit),
     };
 
     let err = helper
@@ -2090,6 +2093,7 @@ fn test_update_config() {
         config.incentivization_fee_info.unwrap(),
         new_incentivization_fee_info
     );
+    assert_eq!(config.token_transfer_gas_limit.unwrap(), new_gas_limit);
 }
 
 #[test]

packages/astroport/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "astroport"
-version = "5.6.0"
+version = "5.7.0"
 authors = ["Astroport"]
 edition = "2021"
 description = "Common Astroport types, queriers and other utils"