diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs index 80589fd5581..40879e70c6f 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/contract.rs @@ -162,18 +162,14 @@ pub fn try_transfer( // Adds the attributes for each path to the response. In prod, the // addtribute add_rate_limit_attributes is a noop results.iter().fold(Ok(response), |acc, result| { - Ok(add_rate_limit_attributes(acc?, result, channel_value)) + Ok(add_rate_limit_attributes(acc?, result)) }) } #[cfg(test)] -pub fn add_rate_limit_attributes( - response: Response, - result: &RateLimit, - channel_value: u128, -) -> Response { +pub fn add_rate_limit_attributes(response: Response, result: &RateLimit) -> Response { let (used_in, used_out) = result.flow.balance(); - let (max_in, max_out) = result.quota.capacity_at(&channel_value); + let (max_in, max_out) = result.quota.capacity(); // These attributes are only added during testing. That way we avoid // calculating these again on prod. // TODO: Figure out how to include these when testing on the go side. @@ -198,11 +194,7 @@ pub fn add_rate_limit_attributes( } #[cfg(not(test))] -pub fn add_rate_limit_attributes( - response: Response, - _result: &RateLimit, - _channel_value: u128, -) -> Response { +pub fn add_rate_limit_attributes(response: Response, _result: &RateLimit) -> Response { response } diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs index c453caa4023..3c4ccdae697 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/integration_tests.rs @@ -288,4 +288,102 @@ mod tests { let cosmos_msg = cw_template_contract.call(msg).unwrap(); let _err = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); } + + #[test] // Tests that the channel value is based on the value at the beginning of the period + fn channel_value_cached() { + let quotas = vec![ + QuotaMsg::new("daily", RESET_TIME_DAILY, 2, 2), + QuotaMsg::new("weekly", RESET_TIME_WEEKLY, 5, 5), + ]; + + let (mut app, cw_template_contract) = proper_instantiate(vec![PathMsg { + channel_id: format!("channel"), + denom: format!("denom"), + quotas, + }]); + + // Sending 1% (half of the daily allowance) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 1, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _res = app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // Sending 3% is now rate limited + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100, + funds: 3, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // Even if the channel value increases, the percentage is calculated based on the value at period start + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 100000, + funds: 3, + }; + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + let _err = app + .execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One day passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_DAILY + 1) + }); + + // New Channel Value world! + + // Sending 1% of a new value (10_000) passes the daily check, cause it + // has expired, but not the weekly check (The value for last week is + // sitll 100, as only 1 day has passed) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg) + .unwrap_err(); + + // ... One week passes + app.update_block(|b| { + b.height += 10; + b.time = b.time.plus_seconds(RESET_TIME_WEEKLY + 1) + }); + + // Sending 1% of a new value should work and set the value for the day at 10_000 + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 10_000, + funds: 100, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + + // If the value magically decreasses. We can still send up to 100 more (1% of 10k) + let msg = ExecuteMsg::SendPacket { + channel_id: format!("channel"), + denom: format!("denom"), + channel_value: 1, + funds: 75, + }; + + let cosmos_msg = cw_template_contract.call(msg).unwrap(); + app.execute(Addr::unchecked(IBC_ADDR), cosmos_msg).unwrap(); + } } diff --git a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs index 97cf961abd5..5ba5c4ced78 100644 --- a/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs +++ b/x/ibc-rate-limit/contracts/rate-limiter/src/state.rs @@ -132,11 +132,20 @@ impl Flow { /// Applies a transfer. If the Flow is expired (now > period_end), it will /// reset it before applying the transfer. - fn apply_transfer(&mut self, direction: &FlowType, funds: u128, now: Timestamp, duration: u64) { + fn apply_transfer( + &mut self, + direction: &FlowType, + funds: u128, + now: Timestamp, + quota: &Quota, + ) -> bool { + let mut expired = false; if self.is_expired(now) { - self.expire(now, duration) + self.expire(now, quota.duration); + expired = true; } self.add_flow(direction.clone(), funds); + expired } } @@ -153,6 +162,7 @@ pub struct Quota { pub max_percentage_send: u32, pub max_percentage_recv: u32, pub duration: u64, + pub channel_value: Option, } impl Quota { @@ -160,11 +170,14 @@ impl Quota { /// total_value) in each direction based on the total value of the denom in /// the channel. The result tuple represents the max capacity when the /// transfer is in directions: (FlowType::In, FlowType::Out) - pub fn capacity_at(&self, total_value: &u128) -> (u128, u128) { - ( - total_value * (self.max_percentage_recv as u128) / 100_u128, - total_value * (self.max_percentage_send as u128) / 100_u128, - ) + pub fn capacity(&self) -> (u128, u128) { + match self.channel_value { + Some(total_value) => ( + total_value * (self.max_percentage_recv as u128) / 100_u128, + total_value * (self.max_percentage_send as u128) / 100_u128, + ), + None => (0, 0), // This should never happen, but ig the channel value is not set, we disallow any transfer + } } } @@ -179,6 +192,7 @@ impl From<&QuotaMsg> for Quota { max_percentage_send: send_recv.0, max_percentage_recv: send_recv.1, duration: msg.duration, + channel_value: None, } } } @@ -207,10 +221,16 @@ impl RateLimit { channel_value: u128, now: Timestamp, ) -> Result { - self.flow - .apply_transfer(direction, funds, now, self.quota.duration); + let expired = self.flow.apply_transfer(direction, funds, now, &self.quota); + println!("EXPIRED: {expired}"); + // Cache the channel value if it has never been set or it has expired. + if self.quota.channel_value.is_none() || expired { + println!("CHANNEL_VALUE OLD: {:?}", self.quota.channel_value); + println!("CHANNEL_VALUE NEW: {}", channel_value); + self.quota.channel_value = Some(channel_value) + } - let (max_in, max_out) = self.quota.capacity_at(&channel_value); + let (max_in, max_out) = self.quota.capacity(); // Return the effects of applying the transfer or an error. match self.flow.exceeds(direction, max_in, max_out) { true => Err(ContractError::RateLimitExceded {