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.

Sign up for GitHub

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

fix(minor-interchain-token-service): skip invariant checks for p2p #687

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+209 −41
Open

fix(minor-interchain-token-service): skip invariant checks for p2p #687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10bed31
fix(minor-interchain-token-service): skip invariant checks for p2p
cjcobb23 Nov 12, 2024
16f8aec
review
cjcobb23 Nov 12, 2024
e9a08da
cleanup
cjcobb23 Nov 12, 2024
9c64ee9
rm comment
cjcobb23 Nov 12, 2024
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
140 changes: 103 additions & 37 deletions contracts/interchain-token-service/src/contract/execute/interceptors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,52 +3,58 @@
use error_stack::{bail, ensure, report, Result, ResultExt};
use router_api::ChainNameRaw;

use super::Error;
use super::{ChainSpecifier, Error};
use crate::state::{self, TokenDeploymentType};
use crate::{DeployInterchainToken, InterchainTransfer, TokenConfig, TokenId, TokenInstance};

pub fn subtract_supply_amount(
pub fn subtract_supply_amount_if_amplifier_chain(
storage: &mut dyn Storage,
chain: &ChainNameRaw,
chain: &ChainSpecifier,
transfer: &InterchainTransfer,
) -> Result<(), Error> {
let mut token = try_load_token_instance(storage, chain.clone(), transfer.token_id)?;
if !chain.is_amplifier_chain {
return Ok(());
}
let mut token = try_load_token_instance(storage, chain.name.clone(), transfer.token_id)?;

token.supply = token
.supply
.checked_sub(transfer.amount)
.change_context_lazy(|| Error::TokenSupplyInvariantViolated {
token_id: transfer.token_id,
chain: chain.clone(),
chain: chain.name.clone(),
})?;

state::save_token_instance(storage, chain.clone(), transfer.token_id, &token)
state::save_token_instance(storage, chain.name.clone(), transfer.token_id, &token)
.change_context(Error::State)
}

pub fn add_supply_amount(
pub fn add_supply_amount_if_amplifier_chain(
storage: &mut dyn Storage,
chain: &ChainNameRaw,
chain: &ChainSpecifier,
transfer: &InterchainTransfer,
) -> Result<(), Error> {
let mut token = try_load_token_instance(storage, chain.clone(), transfer.token_id)?;
if !chain.is_amplifier_chain {
return Ok(());

Check warning on line 38 in contracts/interchain-token-service/src/contract/execute/interceptors.rs

View check run for this annotation

Codecov / codecov/patch

contracts/interchain-token-service/src/contract/execute/interceptors.rs#L38 

Added line #L38 was not covered by tests
}
let mut token = try_load_token_instance(storage, chain.name.clone(), transfer.token_id)?;

token.supply = token
.supply
.checked_add(transfer.amount)
.change_context_lazy(|| Error::TokenSupplyInvariantViolated {
token_id: transfer.token_id,
chain: chain.clone(),
chain: chain.name.clone(),

Check warning on line 47 in contracts/interchain-token-service/src/contract/execute/interceptors.rs

View check run for this annotation

Codecov / codecov/patch

contracts/interchain-token-service/src/contract/execute/interceptors.rs#L47 

Added line #L47 was not covered by tests
})?;

state::save_token_instance(storage, chain.clone(), transfer.token_id, &token)
state::save_token_instance(storage, chain.name.clone(), transfer.token_id, &token)
.change_context(Error::State)
}

pub fn apply_scaling_factor_to_amount(
storage: &dyn Storage,
source_chain: &ChainNameRaw,
destination_chain: &ChainNameRaw,
source_chain: &ChainSpecifier,
destination_chain: &ChainSpecifier,
mut transfer: InterchainTransfer,
) -> Result<InterchainTransfer, Error> {
transfer.amount = destination_amount(
Expand Down Expand Up @@ -186,6 +192,35 @@
.ok_or(report!(Error::TokenNotDeployed { token_id, chain }))
}

fn try_load_token_instance_or_default(
storage: &dyn Storage,
chain: &ChainSpecifier,
token_id: TokenId,
) -> Result<TokenInstance, Error> {
let res = state::may_load_token_instance(storage, chain.name.clone(), token_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Querying here everytime for now will be much cleaner, the additional gas cost isn't a significant concern for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't only query here, we also need to query when doing balance tracking, which shouldn't use this method, since it would then modify the balance for the origin chain

.change_context(Error::State)?
.ok_or(report!(Error::TokenNotDeployed {
token_id,
chain: chain.name.clone(),
}));
if res.is_err() && !chain.is_amplifier_chain {
return try_load_origin_chain_token_instance(storage, token_id);
}
res
Comment on lines +200 to +209
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let res = state::may_load_token_instance(storage, chain.name.clone(), token_id)
.change_context(Error::State)?
.ok_or(report!(Error::TokenNotDeployed {
token_id,
chain: chain.name.clone(),
}));
if res.is_err() && !chain.is_amplifier_chain {
return try_load_origin_chain_token_instance(storage, token_id);
}
res
match state::may_load_token_instance(storage, chain.name.clone(), token_id)
.change_context(Error::State)?
.ok_or(report!(Error::TokenNotDeployed {
token_id,
chain: chain.name.clone(),
})) {
Ok(token) => Ok(token),
Err(_) if !chain.is_amplifier_chain => {
try_load_origin_chain_token_instance(storage, token_id)
}
Err(err) => Err(err),
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment saying that the default is from the original chain.

Comment on lines +200 to +209
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let res = state::may_load_token_instance(storage, chain.name.clone(), token_id)
.change_context(Error::State)?
.ok_or(report!(Error::TokenNotDeployed {
token_id,
chain: chain.name.clone(),
}));
if res.is_err() && !chain.is_amplifier_chain {
return try_load_origin_chain_token_instance(storage, token_id);
}
res
let Some(token_instance) = state::may_load_token_instance(storage, chain.name.clone(), token_id)
.change_context(Error::State)? {
return token_instance
};
ensure!(!chain.is_amplifier_chain, Error::TokenNotDeployed {
token_id,
chain: chain.name.clone(),
});
try_load_origin_chain_token_instance(storage, token_id)

}

fn try_load_origin_chain_token_instance(
storage: &dyn Storage,
token_id: TokenId,
) -> Result<TokenInstance, Error> {
state::may_load_token_config(storage, &token_id)
.change_context(Error::State)?
.ok_or(report!(Error::TokenConfigNotFound(token_id)))
.and_then(|token_config| {
try_load_token_instance(storage, token_config.origin_chain, token_id)
})
}

/// Calculates the destination token decimals.
///
/// The destination chain's token decimals are calculated and saved as following:
Expand Down Expand Up @@ -229,13 +264,14 @@
/// 4) If new_amount is zero, the translation fails.
fn destination_amount(
storage: &dyn Storage,
source_chain: &ChainNameRaw,
destination_chain: &ChainNameRaw,
source_chain: &ChainSpecifier,
destination_chain: &ChainSpecifier,
token_id: TokenId,
source_amount: nonempty::Uint256,
) -> Result<nonempty::Uint256, Error> {
let source_token = try_load_token_instance(storage, source_chain.clone(), token_id)?;
let destination_token = try_load_token_instance(storage, destination_chain.clone(), token_id)?;
let source_token = try_load_token_instance_or_default(storage, source_chain, token_id)?;
let destination_token =
try_load_token_instance_or_default(storage, destination_chain, token_id)?;

let (source_decimals, destination_decimals) =
(source_token.decimals, destination_token.decimals);
Expand All @@ -244,7 +280,7 @@
return Ok(source_amount);
}

let destination_max_uint = state::load_chain_config(storage, destination_chain)
let destination_max_uint = state::load_chain_config(storage, &destination_chain.name)
.change_context(Error::State)?
.truncation
.max_uint;
Expand All @@ -255,8 +291,8 @@
let scaling_factor = Uint256::from_u128(10)
.checked_pow(source_decimals.abs_diff(destination_decimals).into())
.change_context_lazy(|| Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
source_chain: source_chain.name.to_owned(),
destination_chain: destination_chain.name.to_owned(),

Check warning on line 295 in contracts/interchain-token-service/src/contract/execute/interceptors.rs

View check run for this annotation

Codecov / codecov/patch

contracts/interchain-token-service/src/contract/execute/interceptors.rs#L294-L295 

Added lines #L294 - L295 were not covered by tests
amount: source_amount,
})?;
let destination_amount = if source_decimals > destination_decimals {
Expand All @@ -267,24 +303,24 @@
source_amount
.checked_mul(scaling_factor)
.change_context_lazy(|| Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
source_chain: source_chain.name.to_owned(),
destination_chain: destination_chain.name.to_owned(),

Check warning on line 307 in contracts/interchain-token-service/src/contract/execute/interceptors.rs

View check run for this annotation

Codecov / codecov/patch

contracts/interchain-token-service/src/contract/execute/interceptors.rs#L306-L307 

Added lines #L306 - L307 were not covered by tests
amount: source_amount,
})?
};

if destination_amount.gt(&destination_max_uint) {
bail!(Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
source_chain: source_chain.name.to_owned(),
destination_chain: destination_chain.name.to_owned(),
amount: source_amount,
})
}

nonempty::Uint256::try_from(destination_amount).change_context_lazy(|| {
Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
source_chain: source_chain.name.to_owned(),
destination_chain: destination_chain.name.to_owned(),
amount: source_amount,
}
})
Expand Down Expand Up @@ -313,7 +349,7 @@
use router_api::ChainNameRaw;

use super::Error;
use crate::contract::execute::interceptors;
use crate::contract::execute::{interceptors, ChainSpecifier};
use crate::msg::TruncationConfig;
use crate::state::{self, TokenDeploymentType};
use crate::{msg, DeployInterchainToken, InterchainTransfer, TokenInstance};
Expand Down Expand Up @@ -361,8 +397,14 @@

let transfer = assert_ok!(interceptors::apply_scaling_factor_to_amount(
&storage,
&source_chain,
&destination_chain,
&crate::contract::execute::ChainSpecifier {
name: source_chain,
is_amplifier_chain: true
},
&crate::contract::execute::ChainSpecifier {
name: destination_chain,
is_amplifier_chain: true
},
transfer,
));
assert_eq!(
Expand Down Expand Up @@ -414,8 +456,14 @@

let transfer = assert_ok!(interceptors::apply_scaling_factor_to_amount(
&storage,
&source_chain,
&destination_chain,
&ChainSpecifier {
name: source_chain,
is_amplifier_chain: true
},
&ChainSpecifier {
name: destination_chain,
is_amplifier_chain: true
},
transfer,
));
assert_eq!(
Expand Down Expand Up @@ -467,8 +515,14 @@

let transfer = assert_ok!(interceptors::apply_scaling_factor_to_amount(
&storage,
&source_chain,
&destination_chain,
&ChainSpecifier {
name: source_chain,
is_amplifier_chain: true
},
&ChainSpecifier {
name: destination_chain,
is_amplifier_chain: true
},
transfer,
));
assert_eq!(
Expand Down Expand Up @@ -521,8 +575,14 @@
assert_err_contains!(
interceptors::apply_scaling_factor_to_amount(
&storage,
&source_chain,
&destination_chain,
&ChainSpecifier {
name: source_chain,
is_amplifier_chain: true
},
&ChainSpecifier {
name: destination_chain,
is_amplifier_chain: true
},
transfer,
),
Error,
Expand Down Expand Up @@ -574,8 +634,14 @@
assert_err_contains!(
interceptors::apply_scaling_factor_to_amount(
&storage,
&source_chain,
&destination_chain,
&ChainSpecifier {
name: source_chain,
is_amplifier_chain: true
},
&ChainSpecifier {
name: destination_chain,
is_amplifier_chain: true
},
transfer,
),
Error,
Expand Down
37 changes: 34 additions & 3 deletions contracts/interchain-token-service/src/contract/execute/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use axelar_core_std::nexus;
use axelar_wasm_std::{killswitch, nonempty, FnExt, IntoContractError};
use cosmwasm_std::{DepsMut, HexBinary, QuerierWrapper, Response, Storage};
use error_stack::{bail, ensure, report, Result, ResultExt};
Expand Down Expand Up @@ -42,6 +43,9 @@ pub enum Error {
State,
#[error("chain {0} already registered")]
ChainAlreadyRegistered(ChainNameRaw),

#[error("token {0} config not found")]
TokenConfigNotFound(TokenId),
#[error("token {token_id} not deployed on chain {chain}")]
TokenNotDeployed {
token_id: TokenId,
Expand Down Expand Up @@ -72,6 +76,8 @@ pub enum Error {
token_id: TokenId,
chain: ChainNameRaw,
},
#[error("error querying nexus module for chain registration for chain {0}")]
Nexus(ChainNameRaw),
}

/// Executes an incoming ITS message.
Expand Down Expand Up @@ -111,6 +117,7 @@ fn execute_message_on_hub(

let message = apply_to_hub(
deps.storage,
deps.querier,
cc_id.source_chain.clone(),
destination_chain.clone(),
message,
Expand Down Expand Up @@ -141,6 +148,7 @@ fn execute_message_on_hub(

fn apply_to_hub(
storage: &mut dyn Storage,
querier: QuerierWrapper,
source_chain: ChainNameRaw,
destination_chain: ChainNameRaw,
message: Message,
Expand All @@ -150,7 +158,7 @@ fn apply_to_hub(

match message {
Message::InterchainTransfer(transfer) => {
apply_to_transfer(storage, source_chain, destination_chain, transfer)
apply_to_transfer(storage, querier, source_chain, destination_chain, transfer)
.map(Message::InterchainTransfer)?
}
Message::DeployInterchainToken(deploy_token) => {
Expand All @@ -161,20 +169,43 @@ fn apply_to_hub(
.then(Result::Ok)
}

pub struct ChainSpecifier {
pub name: ChainNameRaw,
pub is_amplifier_chain: bool,
}

fn apply_to_transfer(
storage: &mut dyn Storage,
querier: QuerierWrapper,
source_chain: ChainNameRaw,
destination_chain: ChainNameRaw,
transfer: InterchainTransfer,
) -> Result<InterchainTransfer, Error> {
interceptors::subtract_supply_amount(storage, &source_chain, &transfer)?;
// we only do balance tracking for amplifier chains
let client: nexus::Client = client::CosmosClient::new(querier).into();
let source_chain = ChainSpecifier {
name: source_chain.clone(),
is_amplifier_chain: !client
.is_chain_registered(&source_chain.normalize())
.change_context(Error::Nexus(source_chain.clone()))?,
};
let destination_chain = ChainSpecifier {
name: destination_chain.clone(),
is_amplifier_chain: !client
.is_chain_registered(&destination_chain.normalize())
.change_context(Error::Nexus(destination_chain.clone()))?,
};

interceptors::subtract_supply_amount_if_amplifier_chain(storage, &source_chain, &transfer)?;

let transfer = interceptors::apply_scaling_factor_to_amount(
storage,
&source_chain,
&destination_chain,
transfer,
)?;
interceptors::add_supply_amount(storage, &destination_chain, &transfer)?;

interceptors::add_supply_amount_if_amplifier_chain(storage, &destination_chain, &transfer)?;

Ok(transfer)
}
Expand Down
Loading
Loading