Skip to content

Commit

Permalink
Sora review fix (#996)
Browse files Browse the repository at this point in the history
* fix: 🐛 reviewed bugs

* fix: 🐛 benchmark
  • Loading branch information
yooml authored Jul 11, 2023
1 parent 62f9799 commit 08b3f1f
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 42 deletions.
5 changes: 3 additions & 2 deletions pallets/farming/src/boost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl<T: Config> Pallet<T> {
let current_block_number: T::BlockNumber = frame_system::Pallet::<T>::block_number();
boost_pool_info.start_round = current_block_number;
boost_pool_info.round_length = round_length;
boost_pool_info.end_round = current_block_number + round_length;
boost_pool_info.end_round = current_block_number.saturating_add(round_length);
boost_pool_info.total_votes = Zero::zero();
BoostPoolInfos::<T>::set(boost_pool_info);
let _ = BoostVotingPools::<T>::clear(u32::max_value(), None);
Expand Down Expand Up @@ -152,7 +152,8 @@ impl<T: Config> Pallet<T> {
.ok();
let current_block_number: T::BlockNumber = frame_system::Pallet::<T>::block_number();
boost_pool_info.start_round = current_block_number;
boost_pool_info.end_round = current_block_number + boost_pool_info.round_length;
boost_pool_info.end_round =
current_block_number.saturating_add(boost_pool_info.round_length);
boost_pool_info.total_votes = Zero::zero();
Self::deposit_event(Event::RoundStart { round_length: boost_pool_info.round_length });
BoostPoolInfos::<T>::set(boost_pool_info);
Expand Down
52 changes: 40 additions & 12 deletions pallets/farming/src/gauge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,12 @@ where

ensure!(
gauge_pool_info.max_block >=
gauge_info.gauge_stop_block - gauge_info.gauge_start_block + gauge_block,
gauge_info
.gauge_stop_block
.checked_sub(&gauge_info.gauge_start_block)
.ok_or(ArithmeticError::Overflow)?
.checked_add(&gauge_block)
.ok_or(ArithmeticError::Overflow)?,
Error::<T>::GaugeMaxBlockOverflow
);

Expand All @@ -190,17 +195,25 @@ where
let time_factor_a = gauge_value
.saturated_into::<u128>()
.checked_mul(
(gauge_info.gauge_stop_block - current_block_number)
.saturated_into::<u128>(),
(gauge_info
.gauge_stop_block
.checked_sub(&current_block_number)
.ok_or(ArithmeticError::Overflow)?)
.saturated_into::<u128>(),
)
.ok_or(ArithmeticError::Overflow)?;
let time_factor_b = gauge_block
.saturated_into::<u128>()
.checked_mul(
(gauge_value + gauge_info.gauge_amount).saturated_into::<u128>(),
(gauge_value
.checked_add(&gauge_info.gauge_amount)
.ok_or(ArithmeticError::Overflow)?)
.saturated_into::<u128>(),
)
.ok_or(ArithmeticError::Overflow)?;
let incease_total_time_factor = time_factor_a + time_factor_b;
let incease_total_time_factor = time_factor_a
.checked_add(time_factor_b)
.ok_or(ArithmeticError::Overflow)?;
gauge_info.total_time_factor = gauge_info
.total_time_factor
.checked_add(incease_total_time_factor)
Expand All @@ -210,8 +223,10 @@ where
.gauge_amount
.saturated_into::<u128>()
.checked_mul(
(current_block_number - gauge_info.gauge_last_block)
.saturated_into::<u128>(),
(current_block_number
.checked_sub(&gauge_info.gauge_last_block)
.ok_or(ArithmeticError::Overflow)?)
.saturated_into::<u128>(),
)
.ok_or(ArithmeticError::Overflow)?;
gauge_info.latest_time_factor = gauge_info
Expand All @@ -226,7 +241,10 @@ where
.gauge_amount
.checked_add(&gauge_value)
.ok_or(ArithmeticError::Overflow)?;
gauge_info.gauge_stop_block = gauge_info.gauge_stop_block + gauge_block;
gauge_info.gauge_stop_block = gauge_info
.gauge_stop_block
.checked_add(&gauge_block)
.ok_or(ArithmeticError::Overflow)?;

gauge_pool_info.total_time_factor = gauge_pool_info
.total_time_factor
Expand Down Expand Up @@ -274,11 +292,16 @@ where
.gauge_amount
.saturated_into::<u128>()
.checked_mul(
(start_block - gauge_info.gauge_last_block).saturated_into::<u128>(),
(start_block
.checked_sub(&gauge_info.gauge_last_block)
.ok_or(ArithmeticError::Overflow)?)
.saturated_into::<u128>(),
)
.ok_or(ArithmeticError::Overflow)?;
let gauge_rate = Perbill::from_rational(
latest_claimed_time_factor - gauge_info.claimed_time_factor,
latest_claimed_time_factor
.checked_sub(gauge_info.claimed_time_factor)
.ok_or(ArithmeticError::Overflow)?,
gauge_pool_info.total_time_factor,
);
let total_shares =
Expand Down Expand Up @@ -445,11 +468,16 @@ where
.gauge_amount
.saturated_into::<u128>()
.checked_mul(
(start_block - gauge_info.gauge_last_block).saturated_into::<u128>(),
(start_block
.checked_sub(&gauge_info.gauge_last_block)
.ok_or(ArithmeticError::Overflow)?)
.saturated_into::<u128>(),
)
.ok_or(ArithmeticError::Overflow)?;
let gauge_rate = Perbill::from_rational(
latest_claimed_time_factor - gauge_info.claimed_time_factor,
latest_claimed_time_factor
.checked_sub(gauge_info.claimed_time_factor)
.ok_or(ArithmeticError::Overflow)?,
gauge_pool_info.total_time_factor,
);
let total_shares =
Expand Down
11 changes: 6 additions & 5 deletions pallets/farming/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

// Ensure we're `no_std` when compiling for Wasm.
#![cfg_attr(not(feature = "std"), no_std)]
#![allow(deprecated)] // TODO: remove_prefix: Use `clear_prefix` instead

#[cfg(test)]
mod mock;

Expand Down Expand Up @@ -216,6 +214,7 @@ pub mod pallet {
NobodyVoting,
NotInWhitelist,
PercentOverflow,
PoolNotCleared,
}

#[pallet::storage]
Expand Down Expand Up @@ -336,8 +335,8 @@ pub mod pallet {
{
pool_info.block_startup = Some(n);
pool_info.state = PoolState::Ongoing;
PoolInfos::<T>::insert(pid, &pool_info);
}
PoolInfos::<T>::insert(pid, &pool_info);
},
_ => (),
});
Expand Down Expand Up @@ -573,7 +572,8 @@ pub mod pallet {
let share_info = Self::shares_and_withdrawn_rewards(&pid, &exchanger)
.ok_or(Error::<T>::ShareInfoNotExists)?;
ensure!(
share_info.claim_last_block + pool_info.claim_limit_time <= current_block_number,
share_info.claim_last_block.saturating_add(pool_info.claim_limit_time) <=
current_block_number,
Error::<T>::CanNotClaim
);

Expand Down Expand Up @@ -742,7 +742,8 @@ pub mod pallet {
pool_info.state == PoolState::Retired || pool_info.state == PoolState::UnCharged,
Error::<T>::InvalidPoolState
);
SharesAndWithdrawnRewards::<T>::remove_prefix(pid, None);
let res = SharesAndWithdrawnRewards::<T>::clear_prefix(pid, u32::max_value(), None);
ensure!(res.maybe_cursor.is_none(), Error::<T>::PoolNotCleared);
PoolInfos::<T>::remove(pid);

Self::deposit_event(Event::FarmingPoolKilled { pid });
Expand Down
1 change: 0 additions & 1 deletion pallets/slp/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

// Ensure we're `no_std` when compiling for Wasm.
#![recursion_limit = "256"]
#![cfg(feature = "runtime-benchmarks")]

use frame_benchmarking::{account, benchmarks, v1::BenchmarkError, whitelisted_caller};
Expand Down
1 change: 1 addition & 0 deletions pallets/slp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
// along with this program. If not, see <https://www.gnu.org/licenses/>.

#![cfg_attr(not(feature = "std"), no_std)]
#![recursion_limit = "256"]

extern crate core;

Expand Down
24 changes: 22 additions & 2 deletions pallets/system-maker/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@

#![cfg(feature = "runtime-benchmarks")]

use crate::{BalanceOf, Call, Config, Info, Pallet as SystemMaker, Pallet};
use crate::{BalanceOf, Call, Config, Info, Pallet as SystemMaker, Pallet, *};
use frame_benchmarking::v1::{account, benchmarks, BenchmarkError};
use frame_support::{assert_ok, traits::EnsureOrigin};
use frame_support::{
assert_ok,
traits::{EnsureOrigin, Hooks},
};
use frame_system::RawOrigin;
use node_primitives::{CurrencyId, TokenSymbol};
use orml_traits::MultiCurrency;
use sp_core::Get;
use sp_runtime::traits::{AccountIdConversion, UniqueSaturatedFrom};
// use crate::{Pallet as SystemMaker, *};

benchmarks! {
set_config {
Expand Down Expand Up @@ -66,5 +70,21 @@ benchmarks! {
T::MultiCurrency::deposit(CurrencyId::Token(TokenSymbol::DOT), &pallet_account, BalanceOf::<T>::unique_saturated_from(100000000000u128))?;
}: _<T::RuntimeOrigin>(origin,CurrencyId::Token(TokenSymbol::DOT),BalanceOf::<T>::unique_saturated_from(10000000000u128))

on_idle {
let origin = T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
assert_ok!(SystemMaker::<T>::set_config(
T::ControlOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?,
CurrencyId::Token(TokenSymbol::KSM),
Info {
vcurrency_id: CurrencyId::VToken(TokenSymbol::KSM),
annualization: 600_000u32,
granularity: 1000u32.into(),
minimum_redeem: 20000u32.into()
},
));
}: {
SystemMaker::<T>::on_idle(BlockNumberFor::<T>::from(0u32),Weight::from_parts(0, u64::MAX));
}

impl_benchmark_test_suite!(SystemMaker,crate::mock::ExtBuilder::default().build(),crate::mock::Runtime);
}
2 changes: 1 addition & 1 deletion pallets/system-maker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub mod pallet {
Self::handle_redeem_by_currency_id(&system_maker, &info, redeem_amount);
}
}
Weight::zero()
T::WeightInfo::on_idle()
}
}

Expand Down
29 changes: 29 additions & 0 deletions pallets/system-maker/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub trait WeightInfo {
fn charge() -> Weight;
fn close() -> Weight;
fn payout() -> Weight;
fn on_idle() -> Weight;
}

/// Weights for bifrost_system_maker using the Bifrost node and recommended hardware.
Expand Down Expand Up @@ -115,6 +116,20 @@ impl<T: frame_system::Config> WeightInfo for BifrostWeight<T> {
.saturating_add(T::DbWeight::get().reads(5_u64))
.saturating_add(T::DbWeight::get().writes(3_u64))
}
/// Storage: SystemMaker Infos (r:2 w:0)
/// Proof Skipped: SystemMaker Infos (max_values: None, max_size: None, mode: Measured)
/// Storage: ParachainInfo ParachainId (r:1 w:0)
/// Proof: ParachainInfo ParachainId (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen)
/// Storage: Tokens Accounts (r:2 w:0)
/// Proof: Tokens Accounts (max_values: None, max_size: Some(118), added: 2593, mode: MaxEncodedLen)
fn on_idle() -> Weight {
// Proof Size summary in bytes:
// Measured: `709`
// Estimated: `11344`
// Minimum execution time: 591_941 nanoseconds.
Weight::from_parts(594_646_000, 11344)
.saturating_add(T::DbWeight::get().reads(5))
}
}

// For backwards compatibility and tests
Expand Down Expand Up @@ -170,4 +185,18 @@ impl WeightInfo for () {
.saturating_add(RocksDbWeight::get().reads(5_u64))
.saturating_add(RocksDbWeight::get().writes(3_u64))
}
/// Storage: SystemMaker Infos (r:2 w:0)
/// Proof Skipped: SystemMaker Infos (max_values: None, max_size: None, mode: Measured)
/// Storage: ParachainInfo ParachainId (r:1 w:0)
/// Proof: ParachainInfo ParachainId (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen)
/// Storage: Tokens Accounts (r:2 w:0)
/// Proof: Tokens Accounts (max_values: None, max_size: Some(118), added: 2593, mode: MaxEncodedLen)
fn on_idle() -> Weight {
// Proof Size summary in bytes:
// Measured: `709`
// Estimated: `11344`
// Minimum execution time: 591_941 nanoseconds.
Weight::from_parts(594_646_000, 11344)
.saturating_add(RocksDbWeight::get().reads(5))
}
}
27 changes: 19 additions & 8 deletions pallets/ve-minting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ pub mod pallet {
.checked_mul(&T::Week::get())
.ok_or(ArithmeticError::Overflow)?;
for _i in 0..255 {
t_i += T::Week::get();
t_i = t_i.checked_add(&T::Week::get()).ok_or(ArithmeticError::Overflow)?;
let mut d_slope = Zero::zero();
if t_i > current_block_number {
t_i = current_block_number
Expand Down Expand Up @@ -438,7 +438,7 @@ pub mod pallet {

last_checkpoint = t_i;
last_point.block = t_i;
g_epoch += U256::one();
g_epoch = g_epoch.checked_add(U256::one()).ok_or(ArithmeticError::Overflow)?;

// Fill for the current block, if applicable
if t_i == current_block_number {
Expand Down Expand Up @@ -494,7 +494,9 @@ pub mod pallet {
}

// Now handle user history
let user_epoch = Self::user_point_epoch(addr) + U256::one();
let user_epoch = Self::user_point_epoch(addr)
.checked_add(U256::one())
.ok_or(ArithmeticError::Overflow)?;
UserPointEpoch::<T>::insert(addr, user_epoch);
u_new.block = current_block_number;
u_new.amount = Self::locked(addr).amount;
Expand All @@ -512,10 +514,10 @@ pub mod pallet {
let current_block_number: T::BlockNumber = frame_system::Pallet::<T>::block_number();
let mut _locked = locked_balance;
let supply_before = Self::supply();
Supply::<T>::set(supply_before + value);
Supply::<T>::set(supply_before.checked_add(&value).ok_or(ArithmeticError::Overflow)?);

let old_locked = _locked.clone();
_locked.amount += value;
_locked.amount = _locked.amount.checked_add(&value).ok_or(ArithmeticError::Overflow)?;
if unlock_time != Zero::zero() {
_locked.end = unlock_time
}
Expand All @@ -537,7 +539,10 @@ pub mod pallet {
end: _locked.end,
now: current_block_number,
});
Self::deposit_event(Event::Supply { supply_before, supply: supply_before + value });
Self::deposit_event(Event::Supply {
supply_before,
supply: supply_before.checked_add(&value).ok_or(ArithmeticError::Overflow)?,
});
Ok(())
}

Expand Down Expand Up @@ -597,12 +602,18 @@ pub mod pallet {
if _min >= _max {
break;
}
let _mid = (_min + _max + 1) / 2;
let _mid = (_min
.checked_add(_max)
.ok_or(ArithmeticError::Overflow)?
.checked_add(U256::one())
.ok_or(ArithmeticError::Overflow)?)
.checked_div(U256::from(2_u128))
.ok_or(ArithmeticError::Overflow)?;

if Self::user_point_history(addr, _mid).block <= block {
_min = _mid
} else {
_max = _mid - 1
_max = _mid.checked_sub(U256::one()).ok_or(ArithmeticError::Overflow)?
}
}

Expand Down
Loading

0 comments on commit 08b3f1f

Please sign in to comment.