hats-finance / Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430

Apache License 2.0
0 stars 0 forks source link

Duplicate error id from different part of code #2

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1f8723a1839e4aca2cf586aa36fd70ccca364fca72be213b6369662a384c215b Severity: low

Description: Description:

There is this enum of MathError

#[derive(Debug, PartialEq, Eq, scale::Encode, scale::Decode)]
#[cfg_attr(feature = "std", derive(scale_info::TypeInfo))]
pub enum MathError {
    AddOverflow(u8),
    CastOverflow(u8),
    DivByZero(u8),
    MulOverflow(u8),
    SubUnderflow(u8),
    Precision(u8),
}

I'm not sure but, IMO, this enum of MathError might be for an identification, so for example if there is an error exception of DivByZero with some number, we can easily identified which part of code raising the error by checking the id.

Assuming it is for identification, there should be different parameter for each of the enums. Means, for example the DivByZero parameter id should not duplicate.

But if we look at DivByZero, there are two 61 ids, in fees.rs, this should not be the case. Thus this is a Low issue.

File: fees.rs
58:     pub fn normalized_trade_fee(&self, num_coins: u32, amount: u128) -> Result<u128, MathError> {
59:         let adjusted_trade_fee = (self
60:             .trade_fee
61:             .checked_mul(num_coins)
62:             .ok_or(MathError::MulOverflow(61)))?
63:         .checked_div(
64:             (num_coins.checked_sub(1).ok_or(MathError::SubUnderflow(62)))?
65:                 .checked_mul(4)
66:                 .ok_or(MathError::MulOverflow(62))?,
67:         )
68: @>      .ok_or(MathError::DivByZero(61))?;
69:         u128_ratio(amount, adjusted_trade_fee, FEE_DENOM)
70:     }
71: }
72: 
73: fn u128_ratio(amount: u128, num: u32, denom: u32) -> Result<u128, MathError> {
74:     casted_mul(amount, num.into())
75:         .checked_div(denom.into())
76: @>      .ok_or(MathError::DivByZero(61))?
77:         .try_into()
78:         .map_err(|_| MathError::CastOverflow(61))
79: }

Scenario:

Impact:

Wrong logging when there is error exception raised.

Mitigation:

Change one of the 61 id to some other number

JanKuczma commented 2 months ago

Invalid submission: it does not change the intended behavior of the contract