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.

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

281 inaccuracy in approve chain extension behavior #283

Merged
merged 2 commits into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 25 additions & 23 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! Autogenerated weights for `orml_currencies_allowance_extension`
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2023-07-18, STEPS: `100`, REPEAT: `10`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2023-07-19, STEPS: `100`, REPEAT: `10`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `Hugos-MacBook-Pro.local`, CPU: `<UNKNOWN>`
//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("foucoco"), DB CACHE: 1024
Expand Down Expand Up @@ -35,7 +35,7 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 5_000_000 picoseconds.
// Minimum execution time: 6_000_000 picoseconds.
Weight::from_parts(6_000_000, 0)
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
Expand All @@ -46,23 +46,23 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 5_000_000 picoseconds.
// Minimum execution time: 6_000_000 picoseconds.
Weight::from_parts(6_000_000, 0)
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: TokenAllowance AllowedCurrencies (r:1 w:0)
/// Proof Skipped: TokenAllowance AllowedCurrencies (max_values: None, max_size: None, mode: Measured)
/// Storage: TokenAllowance Approvals (r:1 w:1)
/// Storage: TokenAllowance Approvals (r:0 w:1)
/// Proof Skipped: TokenAllowance Approvals (max_values: None, max_size: None, mode: Measured)
fn approve() -> Weight {
// Proof Size summary in bytes:
// Measured: `184`
// Estimated: `7298`
// Estimated: `3833`
// Minimum execution time: 11_000_000 picoseconds.
Weight::from_parts(12_000_000, 0)
.saturating_add(Weight::from_parts(0, 7298))
.saturating_add(T::DbWeight::get().reads(2))
.saturating_add(Weight::from_parts(0, 3833))
.saturating_add(T::DbWeight::get().reads(1))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: TokenAllowance AllowedCurrencies (r:1 w:0)
Expand All @@ -75,8 +75,8 @@ impl<T: frame_system::Config> crate::WeightInfo for WeightInfo<T> {
// Proof Size summary in bytes:
// Measured: `561`
// Estimated: `14248`
// Minimum execution time: 29_000_000 picoseconds.
Weight::from_parts(30_000_000, 0)
// Minimum execution time: 31_000_000 picoseconds.
Weight::from_parts(33_000_000, 0)
.saturating_add(Weight::from_parts(0, 14248))
.saturating_add(T::DbWeight::get().reads(4))
.saturating_add(T::DbWeight::get().writes(3))
Expand Down
20 changes: 6 additions & 14 deletions pallets/orml-currencies-allowance-extension/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use frame_support::{dispatch::DispatchResult, ensure, weights::Weight};
#[cfg(test)]
use mocktopus::macros::mockable;
use orml_traits::MultiCurrency;
use sp_runtime::{traits::*, ArithmeticError};
use sp_runtime::traits::*;
use sp_std::{convert::TryInto, prelude::*, vec};

#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -245,18 +245,7 @@ impl<T: Config> Pallet<T> {
amount: BalanceOf<T>,
) -> DispatchResult {
ensure!(Self::is_allowed_currency(id), Error::<T>::CurrencyNotLive);
Approvals::<T>::try_mutate((id, &owner, &delegate), |maybe_approved| -> DispatchResult {
let mut approved = match maybe_approved.take() {
// an approval already exists and is being updated
Some(a) => a,
// a new approval is created
None => Default::default(),
};

approved = approved.checked_add(&amount).ok_or(ArithmeticError::Overflow)?;
*maybe_approved = Some(approved);
Ok(())
})?;
Approvals::<T>::set((id, &owner, &delegate), Some(amount));
Self::deposit_event(Event::TransferApproved {
currency_id: id,
source: owner.clone(),
Expand Down Expand Up @@ -298,7 +287,10 @@ impl<T: Config> Pallet<T> {
if remaining.is_zero() {
*maybe_approved = None;
} else {
approved = remaining;
//decrement allowance only if it isn't max value (which acts as infinite allowance)
if approved != BalanceOf::<T>::max_value() {
approved = remaining;
}
*maybe_approved = Some(approved);
}
Ok(())
Expand Down
Loading