Merge pull request #4 from neutron-org/fix/audit

audit fixes: remove interchain_tx_in_progress from state

integration-tests/artifacts/scripts/init-neutrond.sh

@@ -10,7 +10,7 @@ THIRD_PARTY_CONTRACTS_DIR=${THIRD_PARTY_CONTRACTS_DIR:-/opt/contracts_thirdparty
 
 # IMPORTANT! minimum_gas_prices should always contain at least one record, otherwise the chain will not start or halt
 # ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2 denom is required by intgration tests (test:tokenomics)
-MIN_GAS_PRICES_DEFAULT='[{"denom":"ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2","amount":"0"},{"denom":"untrn","amount":"0"}]'
+MIN_GAS_PRICES_DEFAULT='[{"denom":"ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2","amount":"0"},{"denom":"untrn","amount":"0"}],'
 MIN_GAS_PRICES=${MIN_GAS_PRICES:-"$MIN_GAS_PRICES_DEFAULT"}
 
 

integration-tests/src/testSuite.ts

@@ -105,26 +105,27 @@ const awaitFirstBlock = async (rpc: string): Promise<void> =>
       }
     }, 20_000);
 
-const awaitNeutronChannels = async (rest: string, rpc: string): Promise<void> =>
-    waitFor(async () => {
-      try {
-        const client = new NeutronClient({
-          apiURL: `http://${rest}`,
-          rpcURL: rpc,
-          prefix: 'neutron',
-        });
-        const res = await client.IbcCoreChannelV1.query.queryChannels();
-        if (res.data.channels.length > 0) {
-          let channels = res.data.channels;
-          if (channels.every((c) => c.state === 'STATE_OPEN')) {
-            return true;
-          }
+const awaitNeutronChannels = async (rest: string, rpc: string): Promise<void> => {
+  const client = new NeutronClient({
+    apiURL: `http://${rest}`,
+    rpcURL: rpc,
+    prefix: 'neutron',
+  });
+  await waitFor(async () => {
+    try {
+      const res = await client.IbcCoreChannelV1.query.queryChannels();
+      if (res.data.channels.length > 0) {
+        let channels = res.data.channels;
+        if (channels.every((c) => c.state === 'STATE_OPEN')) {
+          return true;
         }
-      } catch (e) {
-        console.log('failed to find channels: ' + e.message)
-        return false;
       }
-    }, 60_000);
+    } catch (e) {
+      console.log('failed to find channels: ' + e.message)
+      return false;
+    }
+  }, 60_000);
+}
 
 export const generateWallets = async (): Promise<Record<Keys, string>> =>
     keys.reduce(

integration-tests/testcases/claimer.test.ts

@@ -83,6 +83,7 @@ describe('Test claimer artifact', () => {
             transfer_channel_id: transferChannel.channel_id, // neutron to cosmoshub transfer channel id
             ibc_neutron_denom: ibcDenom,
             ibc_timeout_seconds: 3600 * 5,
+            amount: '14000',
         }, 'credits', 'auto', {
             admin: deployer // want to be able to migrate contract for testing purposes (set low timeout values)
         })
@@ -106,7 +107,7 @@ describe('Test claimer artifact', () => {
 
         await expect(() =>
             client.execute(deployer, claimerAddress, {
-                fund_community_pool: { amount: '9000' },
+                fund_community_pool: { },
             }, 'auto', '', [])
         ).rejects.toThrowError(/ica is not created or open/)
     })
@@ -136,8 +137,8 @@ describe('Test claimer artifact', () => {
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         // expect timeout callback to be called
@@ -173,8 +174,8 @@ describe('Test claimer artifact', () => {
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         // check callbacks
@@ -202,8 +203,8 @@ describe('Test claimer artifact', () => {
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         // balance on contract should be 0 + 2500 refunded timeout fee
@@ -235,13 +236,13 @@ describe('Test claimer artifact', () => {
         const callbackStatesLengthBefore = (await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })).length
 
         await client.execute(deployer, claimerAddress, {
-            fund_community_pool: { amount: '14000' },
+            fund_community_pool: { },
         }, 'auto', 'fund community pool', [{ amount: '8000', denom: 'untrn' }])
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         // check new callback
@@ -274,15 +275,20 @@ describe('Test claimer artifact', () => {
     it('[error] fund community pool', async () => {
         const callbackStatesLengthBefore = (await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })).length
 
+        // migrate to big amount
+        await client.migrate(deployer, claimerAddress, claimerCodeId, {
+            amount: '200000',
+        }, 'auto')
+
         // run step 3 with amount that is too big
         await client.execute(deployer, claimerAddress, {
-            fund_community_pool: { amount: '125000' },
+            fund_community_pool: { },
         }, 'auto', '', [{ amount: '8000', denom: 'untrn' }])
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         const ica = await client.queryContractSmart(claimerAddress, { interchain_account: {} })
@@ -291,19 +297,25 @@ describe('Test claimer artifact', () => {
         const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
         expect(callbackStates.length).toEqual(callbackStatesLengthBefore + 1)
         expect(callbackStates[callbackStates.length - 1].error[0].source_port).toEqual(ica.port_id)
+
+        // returns back original amount
+        // migrate to big amount
+        await client.migrate(deployer, claimerAddress, claimerCodeId, {
+            amount: '14000',
+        }, 'auto')
     })
 
     it('[success] fund community pool', async () => {
         const callbackStatesLengthBefore = (await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })).length
 
         await client.execute(deployer, claimerAddress, {
-            fund_community_pool: { amount: '14000' },
+            fund_community_pool: { },
         }, 'auto', '', [{ amount: '8000', denom: 'untrn' }])
 
         // wait until interchain tx is not in progress
         await waitFor(async () => {
-            const inProgress = await client.queryContractSmart(claimerAddress, { interchain_tx_in_progress: {} })
-            return !inProgress
+            const callbackStates = await client.queryContractSmart(claimerAddress, { ibc_callback_states: {} })
+            return callbackStates.length === callbackStatesLengthBefore + 1
         }, 500000)
 
         // should return callback

src/contract.rs

@@ -21,7 +21,7 @@ use crate::error::ContractError;
 use crate::msg::{ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg};
 use crate::state::{
     Config, IbcCallbackState, InterchainAccount, OpenAckVersion, CONFIG, IBC_CALLBACK_STATES,
-    INTERCHAIN_ACCOUNT, INTERCHAIN_TX_IN_PROGRESS,
+    INTERCHAIN_ACCOUNT,
 };
 
 const ICA_ID: &str = "funder";
@@ -55,10 +55,10 @@ pub fn instantiate(
             transfer_channel_id: msg.transfer_channel_id,
             ibc_neutron_denom: msg.ibc_neutron_denom,
             ibc_timeout_seconds: msg.ibc_timeout_seconds,
+            amount: msg.amount,
         },
     )?;
     INTERCHAIN_ACCOUNT.save(deps.storage, &None)?;
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &false)?;
     IBC_CALLBACK_STATES.save(deps.storage, &vec![])?;
 
     Ok(Response::default())
@@ -71,20 +71,12 @@ pub fn execute(
     info: MessageInfo,
     msg: ExecuteMsg,
 ) -> NeutronResult<Response<NeutronMsg>> {
-    if INTERCHAIN_TX_IN_PROGRESS.load(deps.storage)? {
-        return Err(NeutronError::Std(StdError::generic_err(
-            "interchain transaction is in progress",
-        )));
-    }
-
     match msg {
         ExecuteMsg::CreateHubICA {} => execute_create_hub_ica(deps, env, info),
         ExecuteMsg::SendClaimedTokensToICA {} => {
             execute_send_claimed_tokens_to_ica(deps, env, info)
         }
-        ExecuteMsg::FundCommunityPool { amount } => {
-            execute_fund_community_pool(deps, env, info, amount)
-        }
+        ExecuteMsg::FundCommunityPool {} => execute_fund_community_pool(deps, env, info),
     }
 }
 
@@ -112,8 +104,6 @@ fn execute_send_claimed_tokens_to_ica(
     env: Env,
     info: MessageInfo,
 ) -> NeutronResult<Response<NeutronMsg>> {
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &true)?;
-
     let config = CONFIG.load(deps.storage)?;
     let ica = INTERCHAIN_ACCOUNT.load(deps.storage)?.ok_or_else(|| {
         NeutronError::Std(StdError::generic_err(
@@ -159,9 +149,7 @@ fn execute_fund_community_pool(
     deps: DepsMut<NeutronQuery>,
     _env: Env,
     info: MessageInfo,
-    amount: Uint128,
 ) -> NeutronResult<Response<NeutronMsg>> {
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &true)?;
     let config = CONFIG.load(deps.storage)?;
     let ica = INTERCHAIN_ACCOUNT.load(deps.storage)?.ok_or_else(|| {
         NeutronError::Std(StdError::generic_err(
@@ -171,7 +159,7 @@ fn execute_fund_community_pool(
 
     let coin = CosmosCoin {
         denom: config.ibc_neutron_denom.to_string(),
-        amount: amount.to_string(),
+        amount: config.amount.to_string(),
     };
 
     let ica_msg = MsgFundCommunityPool {
@@ -213,7 +201,6 @@ fn execute_fund_community_pool(
 pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult<Binary> {
     match msg {
         QueryMsg::InterchainAccount {} => query_interchain_account(deps),
-        QueryMsg::InterchainTxInProgress {} => query_interchain_tx_in_progress(deps),
         QueryMsg::IbcCallbackStates {} => query_ibc_callback_states(deps),
     }
 }
@@ -223,11 +210,6 @@ fn query_interchain_account(deps: Deps) -> StdResult<Binary> {
     to_binary(&ica)
 }
 
-fn query_interchain_tx_in_progress(deps: Deps) -> StdResult<Binary> {
-    let ica_in_progress = INTERCHAIN_TX_IN_PROGRESS.load(deps.storage)?;
-    to_binary(&ica_in_progress)
-}
-
 fn query_ibc_callback_states(deps: Deps) -> StdResult<Binary> {
     let states = IBC_CALLBACK_STATES.load(deps.storage)?;
     to_binary(&states)
@@ -262,8 +244,6 @@ fn sudo_response(
     request: RequestPacket,
     _data: Binary,
 ) -> StdResult<Response> {
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &false)?;
-
     save_ibc_callback_state(
         deps.storage,
         IbcCallbackState::Response(request, env.block.height),
@@ -278,8 +258,6 @@ fn sudo_error(
     request: RequestPacket,
     details: String,
 ) -> StdResult<Response> {
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &false)?;
-
     save_ibc_callback_state(
         deps.storage,
         IbcCallbackState::Error(request, details, env.block.height),
@@ -290,8 +268,7 @@ fn sudo_error(
 
 // can be called by response of create ica, ibc transfer and fund community pool
 fn sudo_timeout(deps: DepsMut, env: Env, request: RequestPacket) -> StdResult<Response> {
-    INTERCHAIN_TX_IN_PROGRESS.save(deps.storage, &false)?;
-
+    // save into callback state and cleanup ICA
     let source_port = request
         .source_port
         .clone()
@@ -405,6 +382,11 @@ pub fn migrate(deps: DepsMut, _env: Env, msg: MigrateMsg) -> StdResult<Response>
         if let Some(ibc_timeout_seconds) = msg.ibc_timeout_seconds {
             config.ibc_timeout_seconds = ibc_timeout_seconds;
         }
+
+        if let Some(amount) = msg.amount {
+            config.amount = amount;
+        }
+
         config
     };
     CONFIG.save(deps.storage, &new_config)?;

src/msg.rs

@@ -14,6 +14,9 @@ pub struct InstantiateMsg {
 
     /// relative timeout for ica transactions
     pub ibc_timeout_seconds: u64,
+
+    /// amount of tokens to fund community pool
+    pub amount: Uint128,
 }
 
 #[cw_serde]
@@ -25,7 +28,7 @@ pub enum ExecuteMsg {
     SendClaimedTokensToICA {},
 
     /// Requires ICA to be created and open. Funds cosmoshub community pool with given `amount` of funds.
-    FundCommunityPool { amount: Uint128 },
+    FundCommunityPool {},
 }
 
 #[cw_serde]
@@ -34,9 +37,6 @@ pub enum QueryMsg {
     #[returns(Option<crate::state::InterchainAccount>)]
     InterchainAccount {},
 
-    #[returns(bool)]
-    InterchainTxInProgress {},
-
     #[returns(Vec<crate::state::IbcCallbackState>)]
     IbcCallbackStates {},
 }
@@ -49,4 +49,7 @@ pub struct MigrateMsg {
 
     // ica address to send funds to
     pub ica_address: Option<String>,
+
+    // amount to fund community pool
+    pub amount: Option<Uint128>,
 }

src/state.rs

@@ -1,14 +1,12 @@
 use cosmwasm_schema::cw_serde;
+use cosmwasm_std::Uint128;
 use cw_storage_plus::Item;
 use neutron_sdk::sudo::msg::RequestPacket;
 
 pub const CONFIG: Item<Config> = Item::new("config");
 
 pub const INTERCHAIN_ACCOUNT: Item<Option<InterchainAccount>> = Item::new("interchain_account");
 
-// if true, interchain operation is in progress and we cannot make an operation
-pub const INTERCHAIN_TX_IN_PROGRESS: Item<bool> = Item::new("interchain_tx_in_progress");
-
 // to understand what happened with IBC calls
 pub const IBC_CALLBACK_STATES: Item<Vec<IbcCallbackState>> = Item::new("ibc_callback_states");
 
@@ -18,6 +16,7 @@ pub struct Config {
     pub transfer_channel_id: String,
     pub ibc_neutron_denom: String,
     pub ibc_timeout_seconds: u64,
+    pub amount: Uint128,
 }
 
 #[cw_serde]