Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audit fixes: remove interchain_tx_in_progress from state #4

Merged
merged 2 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration-tests/artifacts/scripts/init-neutrond.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"}


Expand Down
37 changes: 19 additions & 18 deletions integration-tests/src/testSuite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
44 changes: 28 additions & 16 deletions integration-tests/testcases/claimer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand All @@ -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/)
})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: {} })
Expand All @@ -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
Expand Down
38 changes: 10 additions & 28 deletions src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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())
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand Down Expand Up @@ -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),
}
}
Expand All @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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()
Expand Down Expand Up @@ -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)?;
Expand Down
11 changes: 7 additions & 4 deletions src/msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -34,9 +37,6 @@ pub enum QueryMsg {
#[returns(Option<crate::state::InterchainAccount>)]
InterchainAccount {},

#[returns(bool)]
InterchainTxInProgress {},

#[returns(Vec<crate::state::IbcCallbackState>)]
IbcCallbackStates {},
}
Expand All @@ -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>,
}
5 changes: 2 additions & 3 deletions src/state.rs
Original file line number Diff line number Diff line change
@@ -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");

Expand All @@ -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]
Expand Down
Loading