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

refactor(minor-interchain-token-service): change max uint to max bits #760

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+87 −143
Open

refactor(minor-interchain-token-service): change max uint to max bits #760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 0 deletions contracts/interchain-token-service/src/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
pub fn migrate(deps: DepsMut, _env: Env, _msg: Empty) -> Result<Response, ContractError> {
// Implement migration logic if needed

// TODO migrate max uint to max uint bits

Check warning on line 59 in contracts/interchain-token-service/src/contract.rs

View check run for this annotation

Codecov / codecov/patch

contracts/interchain-token-service/src/contract.rs#L58-L59 

Added lines #L58 - L59 were not covered by tests
cw2::set_contract_version(deps.storage, CONTRACT_NAME, CONTRACT_VERSION)?;

Ok(Response::default())
Expand Down
40 changes: 25 additions & 15 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
@@ -1,5 +1,5 @@
use axelar_wasm_std::{nonempty, FnExt};
use cosmwasm_std::{Storage, Uint256};
use cosmwasm_std::{OverflowError, Storage, Uint256};
use error_stack::{bail, ensure, report, Result, ResultExt};
use router_api::ChainNameRaw;

Expand Down Expand Up @@ -204,8 +204,8 @@ fn destination_token_decimals(

if source_chain_config
.truncation
.max_uint
.le(&destination_chain_config.truncation.max_uint)
.max_uint_bits
.le(&destination_chain_config.truncation.max_uint_bits)
{
source_token_decimals
} else {
Expand Down Expand Up @@ -242,10 +242,10 @@ fn destination_amount(
return Ok(source_amount);
}

let destination_max_uint = state::load_chain_config(storage, destination_chain)
let destination_max_uint_bits = state::load_chain_config(storage, destination_chain)
.change_context(Error::State)?
.truncation
.max_uint;
.max_uint_bits;

// It's intentionally written in this way since the end result may still be fine even if
// 1) amount * (10 ^ (dest_chain_decimals)) overflows
Expand All @@ -271,7 +271,7 @@ fn destination_amount(
})?
};

if destination_amount.gt(&destination_max_uint) {
if amount_overflows(destination_amount, destination_max_uint_bits) {
bail!(Error::InvalidTransferAmount {
source_chain: source_chain.to_owned(),
destination_chain: destination_chain.to_owned(),
Expand All @@ -288,6 +288,16 @@ fn destination_amount(
})
}

fn amount_overflows(amount: Uint256, target_chain_max_bits: u32) -> bool {
match amount.checked_shr(target_chain_max_bits) {
Ok(res) => res.gt(&Uint256::zero()),
// this overflow error occurs when trying to shift 256 bits or more.
// But this can only happen if max_bits is >= 256, and amount itself is only 256 bits.
// So in this can, amount cannot possibly overflow the max uint of the target chain
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
// So in this can, amount cannot possibly overflow the max uint of the target chain
// So in this case, amount cannot possibly overflow the max uint of the target chain

Err(OverflowError { operation: _ }) => false,
}
}

pub fn register_custom_token(
storage: &mut dyn Storage,
source_chain: ChainNameRaw,
Expand Down Expand Up @@ -414,7 +424,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000u128).try_into().unwrap(),
max_uint_bits: 32u32,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -467,7 +477,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000_000_000u128).try_into().unwrap(),
max_uint_bits: 64,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -520,7 +530,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000_000_000u128).try_into().unwrap(),
max_uint_bits: 64,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -573,7 +583,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(100_000u128).try_into().unwrap(),
max_uint_bits: 10,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -626,7 +636,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(100_000u128).try_into().unwrap(),
max_uint_bits: 10,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -658,7 +668,7 @@ mod test {
chain: source_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000_000_000u128).try_into().unwrap(),
max_uint_bits: 256,
max_decimals_when_truncating: 12,
},
},
Expand All @@ -671,7 +681,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000u128).try_into().unwrap(),
max_uint_bits: 128,
max_decimals_when_truncating: 6,
},
},
Expand Down Expand Up @@ -722,7 +732,7 @@ mod test {
chain: source_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000u128).try_into().unwrap(),
max_uint_bits: 128,
max_decimals_when_truncating: 6,
},
},
Expand All @@ -735,7 +745,7 @@ mod test {
chain: destination_chain.clone(),
its_edge_contract: "itsedgecontract".to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::from(1_000_000_000_000_000u128).try_into().unwrap(),
max_uint_bits: 256,
max_decimals_when_truncating: 6,
},
},
Expand Down
18 changes: 9 additions & 9 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
Expand Up @@ -430,7 +430,7 @@ mod tests {
use axelarnet_gateway::msg::QueryMsg;
use cosmwasm_std::testing::{mock_dependencies, MockApi, MockQuerier};
use cosmwasm_std::{
from_json, to_json_binary, HexBinary, MemoryStorage, OwnedDeps, Uint128, Uint256, WasmQuery,
from_json, to_json_binary, HexBinary, MemoryStorage, OwnedDeps, Uint256, WasmQuery,
};
use router_api::{ChainName, ChainNameRaw, CrossChainId};

Expand Down Expand Up @@ -681,7 +681,7 @@ mod tests {
chain: SOLANA.parse().unwrap(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::one().try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8
}
}
Expand All @@ -693,7 +693,7 @@ mod tests {
chain: SOLANA.parse().unwrap(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::one().try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8
}
}
Expand All @@ -711,15 +711,15 @@ mod tests {
chain: SOLANA.parse().unwrap(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8,
},
},
msg::ChainConfig {
chain: XRPL.parse().unwrap(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8,
},
},
Expand All @@ -742,7 +742,7 @@ mod tests {
chain: SOLANA.parse().unwrap(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8,
},
}]
Expand All @@ -769,7 +769,7 @@ mod tests {
chain: solana.clone(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint128::MAX.try_into().unwrap(),
max_uint_bits: 128,
max_decimals_when_truncating: new_decimals,
},
}]
Expand Down Expand Up @@ -877,7 +877,7 @@ mod tests {
chain: solana.clone(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint128::MAX.try_into().unwrap(),
max_uint_bits: 128,
max_decimals_when_truncating: 6u8,
},
}]
Expand Down Expand Up @@ -1310,7 +1310,7 @@ mod tests {
chain: chain.clone(),
its_edge_contract: ITS_ADDRESS.to_string().try_into().unwrap(),
truncation: TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 18u8
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/interchain-token-service/src/contract/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn its_chain(deps: Deps, chain: ChainNameRaw) -> Result<Binary, Error> {
chain,
its_edge_contract: config.its_address,
truncation: msg::TruncationConfig {
max_uint: config.truncation.max_uint,
max_uint_bits: config.truncation.max_uint_bits,
max_decimals_when_truncating: config.truncation.max_decimals_when_truncating,
},
frozen: config.frozen,
Expand Down
5 changes: 2 additions & 3 deletions contracts/interchain-token-service/src/msg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::collections::HashMap;

use axelar_wasm_std::nonempty;
use axelarnet_gateway::AxelarExecutableMsg;
use cosmwasm_schema::{cw_serde, QueryResponses};
use msgs_derive::EnsurePermissions;
Expand Down Expand Up @@ -60,8 +59,8 @@ pub struct ChainConfig {

#[cw_serde]
pub struct TruncationConfig {
pub max_uint: nonempty::Uint256, // The maximum uint value that is supported by the chain's token standard
pub max_decimals_when_truncating: u8, // The maximum number of decimals that is preserved when deploying from a chain with a larger max_uint
pub max_uint_bits: u32, // The maximum number of bits used by the chain to represent unsigned integers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a nonempty::u32? zero wouldn't make sense here.

pub max_decimals_when_truncating: u8, // The maximum number of decimals that is preserved when deploying from a chain with a larger max unsigned integer
}

#[cw_serde]
Expand Down
10 changes: 5 additions & 5 deletions contracts/interchain-token-service/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ pub struct ChainConfig {

#[cw_serde]
pub struct TruncationConfig {
pub max_uint: nonempty::Uint256, // The maximum uint value that is supported by the chain's token standard
pub max_decimals_when_truncating: u8, // The maximum number of decimals that is preserved when deploying from a chain with a larger max_uint
pub max_uint_bits: u32, // The maximum number of bits used to represent unsigned integer values that is supported by the chain's token standard
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

pub max_decimals_when_truncating: u8, // The maximum number of decimals that is preserved when deploying from a chain with a larger max unsigned integer
}

impl From<msg::ChainConfig> for ChainConfig {
fn from(value: msg::ChainConfig) -> Self {
Self {
truncation: TruncationConfig {
max_uint: value.truncation.max_uint,
max_uint_bits: value.truncation.max_uint_bits,
max_decimals_when_truncating: value.truncation.max_decimals_when_truncating,
},
its_address: value.its_edge_contract,
Expand Down Expand Up @@ -360,7 +360,7 @@ mod tests {
chain: chain1.clone(),
its_edge_contract: address1.clone(),
truncation: msg::TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8
}
}
Expand All @@ -372,7 +372,7 @@ mod tests {
chain: chain2.clone(),
its_edge_contract: address2.clone(),
truncation: msg::TruncationConfig {
max_uint: Uint256::MAX.try_into().unwrap(),
max_uint_bits: 256u32,
max_decimals_when_truncating: 16u8
}
}
Expand Down
Loading
Loading