Merge pull request #685 from CosmWasm/add-default-gas-limit

Add default gas limit to cw20-ics20

Author: ethanfrey

contracts/cw20-ics20/src/contract.rs

@@ -38,6 +38,7 @@ pub fn instantiate(
     set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
     let cfg = Config {
         default_timeout: msg.default_timeout,
+        default_gas_limit: msg.default_gas_limit,
     };
     CONFIG.save(deps.storage, &cfg)?;
 
@@ -107,19 +108,23 @@ pub fn execute_transfer(
     if !CHANNEL_INFO.has(deps.storage, &msg.channel) {
         return Err(ContractError::NoSuchChannel { id: msg.channel });
     }
+    let config = CONFIG.load(deps.storage)?;
 
-    // if cw20 token, ensure it is whitelisted
+    // if cw20 token, validate and ensure it is whitelisted, or we set default gas limit
     if let Amount::Cw20(coin) = &amount {
         let addr = deps.api.addr_validate(&coin.address)?;
-        ALLOW_LIST
-            .may_load(deps.storage, &addr)?
-            .ok_or(ContractError::NotOnAllowList)?;
+        // if limit is set, then we always allow cw20
+        if config.default_gas_limit.is_none() {
+            ALLOW_LIST
+                .may_load(deps.storage, &addr)?
+                .ok_or(ContractError::NotOnAllowList)?;
+        }
     };
 
     // delta from user is in seconds
     let timeout_delta = match msg.timeout {
         Some(t) => t,
-        None => CONFIG.load(deps.storage)?.default_timeout,
+        None => config.default_timeout,
     };
     // timeout is in nanoseconds
     let timeout = env.block.time.plus_seconds(timeout_delta);
@@ -202,7 +207,7 @@ const MIGRATE_VERSION_2: &str = "0.12.0-alpha1";
 const MIGRATE_VERSION_3: &str = "0.13.1";
 
 #[cfg_attr(not(feature = "library"), entry_point)]
-pub fn migrate(mut deps: DepsMut, env: Env, _msg: MigrateMsg) -> Result<Response, ContractError> {
+pub fn migrate(mut deps: DepsMut, env: Env, msg: MigrateMsg) -> Result<Response, ContractError> {
     let version: Version = CONTRACT_VERSION.parse().map_err(from_semver)?;
     let stored = get_contract_version(deps.storage)?;
     let storage_version: Version = stored.version.parse().map_err(from_semver)?;
@@ -233,6 +238,7 @@ pub fn migrate(mut deps: DepsMut, env: Env, _msg: MigrateMsg) -> Result<Response
         ADMIN.set(deps.branch(), Some(old_config.gov_contract))?;
         let config = Config {
             default_timeout: old_config.default_timeout,
+            default_gas_limit: None,
         };
         CONFIG.save(deps.storage, &config)?;
     }
@@ -242,6 +248,15 @@ pub fn migrate(mut deps: DepsMut, env: Env, _msg: MigrateMsg) -> Result<Response
     }
     // otherwise no migration (yet) - add them here
 
+    // always allow setting the default gas limit via MigrateMsg, even if same version
+    // (Note this doesn't allow unsetting it now)
+    if msg.default_gas_limit.is_some() {
+        CONFIG.update(deps.storage, |mut old| -> StdResult<_> {
+            old.default_gas_limit = msg.default_gas_limit;
+            Ok(old)
+        })?;
+    }
+
     // we don't need to save anything if migrating from the same version
     if storage_version < version {
         set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;
@@ -313,6 +328,7 @@ fn query_config(deps: Deps) -> StdResult<ConfigResponse> {
     let admin = ADMIN.get(deps)?.unwrap_or_else(|| Addr::unchecked(""));
     let res = ConfigResponse {
         default_timeout: cfg.default_timeout,
+        default_gas_limit: cfg.default_gas_limit,
         gov_contract: admin.into(),
     };
     Ok(res)
@@ -512,9 +528,9 @@ mod test {
     }
 
     #[test]
-    fn execute_cw20_fails_if_not_whitelisted() {
+    fn execute_cw20_fails_if_not_whitelisted_unless_default_gas_limit() {
         let send_channel = "channel-15";
-        let mut deps = setup(&["channel-3", send_channel], &[]);
+        let mut deps = setup(&[send_channel], &[]);
 
         let cw20_addr = "my-token";
         let transfer = TransferMsg {
@@ -528,10 +544,23 @@ mod test {
             msg: to_binary(&transfer).unwrap(),
         });
 
-        // works with proper funds
+        // rejected as not on allow list
         let info = mock_info(cw20_addr, &[]);
-        let err = execute(deps.as_mut(), mock_env(), info, msg).unwrap_err();
+        let err = execute(deps.as_mut(), mock_env(), info.clone(), msg.clone()).unwrap_err();
         assert_eq!(err, ContractError::NotOnAllowList);
+
+        // add a default gas limit
+        migrate(
+            deps.as_mut(),
+            mock_env(),
+            MigrateMsg {
+                default_gas_limit: Some(123456),
+            },
+        )
+        .unwrap();
+
+        // try again
+        execute(deps.as_mut(), mock_env(), info, msg).unwrap();
     }
 
     #[test]
@@ -558,11 +587,22 @@ mod test {
             .unwrap();
 
         // run migration
-        migrate(deps.as_mut(), mock_env(), MigrateMsg {}).unwrap();
+        migrate(
+            deps.as_mut(),
+            mock_env(),
+            MigrateMsg {
+                default_gas_limit: Some(123456),
+            },
+        )
+        .unwrap();
 
         // check new channel state
         let chan = query_channel(deps.as_ref(), send_channel.into()).unwrap();
         assert_eq!(chan.balances, vec![Amount::native(50000, native)]);
         assert_eq!(chan.total_sent, vec![Amount::native(114000, native)]);
+
+        // check config updates
+        let config = query_config(deps.as_ref()).unwrap();
+        assert_eq!(config.default_gas_limit, Some(123456));
     }
 }

contracts/cw20-ics20/src/ibc.rs

@@ -12,7 +12,7 @@ use crate::amount::Amount;
 use crate::error::{ContractError, Never};
 use crate::state::{
     reduce_channel_balance, undo_reduce_channel_balance, ChannelInfo, ReplyArgs, ALLOW_LIST,
-    CHANNEL_INFO, REPLY_ARGS,
+    CHANNEL_INFO, CONFIG, REPLY_ARGS,
 };
 use cw20::Cw20ExecuteMsg;
 
@@ -273,10 +273,14 @@ fn check_gas_limit(deps: Deps, amount: &Amount) -> Result<Option<u64>, ContractE
         Amount::Cw20(coin) => {
             // if cw20 token, use the registered gas limit, or error if not whitelisted
             let addr = deps.api.addr_validate(&coin.address)?;
-            Ok(ALLOW_LIST
-                .may_load(deps.storage, &addr)?
-                .ok_or(ContractError::NotOnAllowList)?
-                .gas_limit)
+            let allowed = ALLOW_LIST.may_load(deps.storage, &addr)?;
+            match allowed {
+                Some(allow) => Ok(allow.gas_limit),
+                None => match CONFIG.load(deps.storage)?.default_gas_limit {
+                    Some(base) => Ok(Some(base)),
+                    None => Err(ContractError::NotOnAllowList),
+                },
+            }
         }
         _ => Ok(None),
     }
@@ -386,8 +390,8 @@ mod test {
     use super::*;
     use crate::test_helpers::*;
 
-    use crate::contract::{execute, query_channel};
-    use crate::msg::{ExecuteMsg, TransferMsg};
+    use crate::contract::{execute, migrate, query_channel};
+    use crate::msg::{ExecuteMsg, MigrateMsg, TransferMsg};
     use cosmwasm_std::testing::{mock_env, mock_info};
     use cosmwasm_std::{coins, to_vec, IbcEndpoint, IbcMsg, IbcTimeout, Timestamp};
     use cw20::Cw20ReceiveMsg;
@@ -621,4 +625,39 @@ mod test {
         assert_eq!(state.balances, vec![Amount::native(111111111, denom)]);
         assert_eq!(state.total_sent, vec![Amount::native(987654321, denom)]);
     }
+
+    #[test]
+    fn check_gas_limit_handles_all_cases() {
+        let send_channel = "channel-9";
+        let allowed = "foobar";
+        let allowed_gas = 777666;
+        let mut deps = setup(&[send_channel], &[(allowed, allowed_gas)]);
+
+        // allow list will get proper gas
+        let limit = check_gas_limit(deps.as_ref(), &Amount::cw20(500, allowed)).unwrap();
+        assert_eq!(limit, Some(allowed_gas));
+
+        // non-allow list will error
+        let random = "tokenz";
+        check_gas_limit(deps.as_ref(), &Amount::cw20(500, random)).unwrap_err();
+
+        // add default_gas_limit
+        let def_limit = 54321;
+        migrate(
+            deps.as_mut(),
+            mock_env(),
+            MigrateMsg {
+                default_gas_limit: Some(def_limit),
+            },
+        )
+        .unwrap();
+
+        // allow list still gets proper gas
+        let limit = check_gas_limit(deps.as_ref(), &Amount::cw20(500, allowed)).unwrap();
+        assert_eq!(limit, Some(allowed_gas));
+
+        // non-allow list will now get default
+        let limit = check_gas_limit(deps.as_ref(), &Amount::cw20(500, random)).unwrap();
+        assert_eq!(limit, Some(def_limit));
+    }
 }

contracts/cw20-ics20/src/msg.rs

@@ -14,6 +14,9 @@ pub struct InitMsg {
     pub gov_contract: String,
     /// initial allowlist - all cw20 tokens we will send must be previously allowed by governance
     pub allowlist: Vec<AllowMsg>,
+    /// If set, contracts off the allowlist will run with this gas limit.
+    /// If unset, will refuse to accept any contract off the allow list.
+    pub default_gas_limit: Option<u64>,
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
@@ -23,7 +26,9 @@ pub struct AllowMsg {
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema)]
-pub struct MigrateMsg {}
+pub struct MigrateMsg {
+    pub default_gas_limit: Option<u64>,
+}
 
 #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
@@ -98,6 +103,7 @@ pub struct PortResponse {
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
 pub struct ConfigResponse {
     pub default_timeout: u64,
+    pub default_gas_limit: Option<u64>,
     pub gov_contract: String,
 }
 

contracts/cw20-ics20/src/state.rs

@@ -32,6 +32,7 @@ pub struct ChannelState {
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]
 pub struct Config {
     pub default_timeout: u64,
+    pub default_gas_limit: Option<u64>,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]

contracts/cw20-ics20/src/test_helpers.rs

@@ -70,6 +70,7 @@ pub fn setup(
 
     // instantiate an empty contract
     let instantiate_msg = InitMsg {
+        default_gas_limit: None,
         default_timeout: DEFAULT_TIMEOUT,
         gov_contract: "gov".to_string(),
         allowlist,