Skip to content

Commit

Permalink
doc: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
milapsheth committed Nov 5, 2024
1 parent d1b9ec6 commit 8881f24
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 3 deletions.
14 changes: 11 additions & 3 deletions contracts/interchain-token-service/src/contract/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ pub fn set_chain_config(
/// The amount is calculated based on the token decimals on the source and destination chains.
/// The calculation is done as following:
/// 1) `destination_amount` = `source_amount` * 10 ^ (`destination_chain_decimals` - `source_chain_decimals`)
/// 3) If new_amount is greater than the destination chain's `max_uint`, the translation
/// 3) If `destination_amount` is greater than the destination chain's `max_uint`, the translation
/// fails.
/// 4) If new_amount is zero, the translation fails.
/// 4) If `destination_amount` is zero, the translation fails.
fn destination_amount(
storage: &dyn Storage,
source_chain: &ChainNameRaw,
Expand Down Expand Up @@ -312,6 +312,8 @@ fn destination_amount(
amount: source_amount,
})?;
let destination_amount = if source_decimals > destination_decimals {
// Note: We should track the truncated dust in the global token config to allow recovery in the future
// The token's decimals on the origin chain will always be greater than or equal to `source_decimals`, so we can scale and add to the total dust amount
source_amount
.checked_div(scaling_factor)
.expect("scaling_factor must be non-zero")
Expand All @@ -325,7 +327,9 @@ fn destination_amount(
})?
};

// Note: Use ensure! instead of bail!?
if destination_amount.gt(&destination_max_uint) {
// Note: shall we different error enums to make debugging easier? Maybe add scaling factor in the error as well?
bail!(Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
Expand Down Expand Up @@ -366,7 +370,9 @@ fn destination_token_decimals(
{
source_chain_decimals
} else {
source_chain_config
// Note: Since destination_chain's `max_uint` is less than source_chain's, we need to use `max_target_decimals` of the destination chain.
// E.g. Deploying token from Ethereum to Sui
destination_chain_config
.max_target_decimals
.min(source_chain_decimals)
}
Expand All @@ -379,6 +385,8 @@ fn apply_transfer(
destination_chain: ChainNameRaw,
transfer: &InterchainTransfer,
) -> Result<InterchainTransfer, Error> {
// Note: We're loading the token instances multiple times, are we refactoring it to load first and save at the end?
// Same for chain config (checking frozen status and reading max_uint)
let destination_amount = destination_amount(
storage,
&source_chain,
Expand Down
2 changes: 2 additions & 0 deletions contracts/interchain-token-service/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ impl TokenSupply {
#[cw_serde]
pub struct TokenInstance {
pub supply: TokenSupply,
// Note: Since we have decided to drop deploy token manager msg type, we can save `u8` instead of `Option<u8>`
// When we add custom token linking, we'll include decimals in the msg type
pub decimals: Option<u8>,
}

Expand Down
1 change: 1 addition & 0 deletions contracts/interchain-token-service/tests/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ fn execute_message_interchain_transfer_should_scale_the_amount_when_source_decim
);
}

// Note: we should add a test with 3 chains with distinct max uint values and try a round robin deploy + A -> B -> C transfer test
#[test]
fn execute_message_deploy_interchain_token_should_translate_decimals_when_max_uints_are_different()
{
Expand Down

0 comments on commit 8881f24

Please sign in to comment.