Skip to content

Commit

Permalink
clear votes with tests (#289)
Browse files Browse the repository at this point in the history
* Eoa field (#287)

* new types

* ensure supported currency method

* brief migration, fixes

* fixes

* fmt

* fix up benchmarks

* fix brief test

* update imbue image for rococo

* clear votes with tests

* fix merge

* fix

* fix

* fmt

* fix

---------

Co-authored-by: samelamin <hussam.elamin@gmail.com>
  • Loading branch information
f-gate and samelamin authored Dec 8, 2023
1 parent bfa42bf commit 7e99ad5
Show file tree
Hide file tree
Showing 23 changed files with 413 additions and 70 deletions.
3 changes: 3 additions & 0 deletions libs/common-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ std = [
'sp-std/std',
'xcm/std',
]

runtime-benchmarks = []

try-runtime = [
"common-traits/try-runtime",
"frame-support/try-runtime",
Expand Down
83 changes: 73 additions & 10 deletions libs/common-types/src/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,79 @@ pub enum CurrencyId {
ForeignAsset(ForeignAssetId),
}

#[derive(
Clone,
Copy,
PartialOrd,
Ord,
PartialEq,
Eq,
Debug,
Encode,
Decode,
TypeInfo,
MaxEncodedLen,
Serialize,
Deserialize,
)]
pub enum ForeignAssetId {
ETH,
USDT,
}

#[derive(
Clone,
Copy,
PartialOrd,
Ord,
PartialEq,
Eq,
Debug,
Encode,
Decode,
TypeInfo,
MaxEncodedLen,
Serialize,
Deserialize,
)]
/// The foreign owned account describes the chain
pub enum ForeignOwnedAccount {
TRON([u8; 22]),
ETH([u8; 20]),
}

impl ForeignOwnedAccount {
/// Here we can define which currencies per network we support
/// For example when given a TRON account we can use this to see if the account
/// and the currency are compatible.
pub fn ensure_supported_currency(&self, currency: CurrencyId) -> bool {
match currency {
CurrencyId::Native => false,
CurrencyId::KSM => false,
CurrencyId::AUSD => false,
CurrencyId::KAR => false,
CurrencyId::MGX => false,
CurrencyId::ForeignAsset(asset) => match &self {
ForeignOwnedAccount::TRON(_) => match asset {
ForeignAssetId::ETH => false,
ForeignAssetId::USDT => true,
},
ForeignOwnedAccount::ETH(_) => match asset {
ForeignAssetId::ETH => true,
ForeignAssetId::USDT => true,
},
},
}
}
#[cfg(feature = "runtime-benchmarks")]
pub fn get_supported_currency_eoa_combo() -> (ForeignOwnedAccount, CurrencyId) {
(
ForeignOwnedAccount::ETH(Default::default()),
CurrencyId::ForeignAsset(ForeignAssetId::ETH),
)
}
}

pub mod currency_decimals {
pub const NATIVE: u32 = 12;
pub const AUSD: u32 = 12;
Expand All @@ -39,16 +112,6 @@ pub mod currency_decimals {
pub const MGX: u32 = 18;
}

// A way to generate different currencies from a number.
// Can be used in tests/benchmarks to generate different currencies.
impl From<ForeignAssetId> for CurrencyId {
fn from(value: ForeignAssetId) -> Self {
CurrencyId::ForeignAsset(value)
}
}

pub type ForeignAssetId = u32;

#[derive(
Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Debug, Encode, Decode, TypeInfo, MaxEncodedLen,
)]
Expand Down
1 change: 1 addition & 0 deletions pallets/briefs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ std = [

runtime-benchmarks = [
"common-runtime/runtime-benchmarks",
"common-types/runtime-benchmarks",
"frame-benchmarking/runtime-benchmarks",
"pallet-xcm/runtime-benchmarks",
"pallet-deposits/runtime-benchmarks",
Expand Down
53 changes: 33 additions & 20 deletions pallets/briefs/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::*;
use crate::test_utils::gen_hash;
use crate::Pallet as Briefs;
use crate::{BoundedBriefOwners, BoundedProposedMilestones};
use common_types::CurrencyId;
use common_types::{CurrencyId, ForeignOwnedAccount};
use frame_benchmarking::v2::*;
use frame_support::{assert_ok, traits::Get};
use frame_system::{EventRecord, RawOrigin};
Expand All @@ -22,14 +22,15 @@ mod benchmarks {

#[benchmark]
fn create_brief() {
let brief_owners = get_max_brief_owners::<T>();
let (eoa, currency_id) = ForeignOwnedAccount::get_supported_currency_eoa_combo();
let brief_owners = get_max_brief_owners::<T>(currency_id);
let caller: T::AccountId = brief_owners[0].clone();
let applicant = create_account_id::<T>("applicant", 1);
let applicant = create_account_id::<T>("applicant", 1, currency_id);
let budget = 10_000u32.into();
let initial_contribution = 5_000u32.into();
let brief_id = gen_hash(1);
let milestones = get_max_milestones::<T>();
// (origin, brief_owners, applicant, budget, initial_contribution, brief_id, currency_id, milestones)
// (origin, brief_owners, applicant, budget, initial_contribution, brief_id, currency_id, milestones, Option<eoa>)

#[extrinsic_call]
create_brief(
Expand All @@ -39,18 +40,20 @@ mod benchmarks {
budget,
initial_contribution,
brief_id,
CurrencyId::Native,
currency_id,
milestones,
Some(eoa),
false,
);
assert_last_event::<T>(Event::<T>::BriefSubmitted(caller, brief_id).into());
}

#[benchmark]
fn contribute_to_brief() {
let brief_owners = get_max_brief_owners::<T>();
let currency_id = CurrencyId::Native;
let brief_owners = get_max_brief_owners::<T>(currency_id);
let caller: T::AccountId = brief_owners[0].clone();
let applicant: T::AccountId = create_account_id::<T>("applicant", 1);
let applicant: T::AccountId = create_account_id::<T>("applicant", 1, currency_id);
let budget = 10_000_000_000_000u128.saturated_into();
let initial_contribution = 5_000_000_000_000u128.saturated_into();
let contribution = 5_000_000_000_000u128.saturated_into();
Expand All @@ -63,8 +66,9 @@ mod benchmarks {
budget,
initial_contribution,
brief_id,
CurrencyId::Native,
currency_id,
milestones,
None,
false,
));
let brief_owner: T::AccountId = brief_owners[0].clone();
Expand All @@ -80,22 +84,25 @@ mod benchmarks {

#[benchmark]
fn commence_work() {
let brief_owners = get_max_brief_owners::<T>();
let currency_id = CurrencyId::Native;
let brief_owners = get_max_brief_owners::<T>(currency_id);
let caller: T::AccountId = brief_owners[0].clone();
let applicant: T::AccountId = create_account_id::<T>("applicant", 1);
let applicant: T::AccountId = create_account_id::<T>("applicant", 1, currency_id);
let budget = 10_000_000_000_000u128.saturated_into();
let initial_contribution = 5_000_000_000_000u128.saturated_into();
let brief_id = gen_hash(1);
let milestones = get_max_milestones::<T>();

assert_ok!(Briefs::<T>::create_brief(
RawOrigin::Signed(caller).into(),
brief_owners,
applicant.clone(),
budget,
initial_contribution,
brief_id,
CurrencyId::Native,
currency_id,
milestones,
None,
false,
));
// (origin, brief_id)
Expand All @@ -106,9 +113,10 @@ mod benchmarks {

#[benchmark]
fn cancel_brief() {
let brief_owners = get_max_brief_owners::<T>();
let currency_id = CurrencyId::Native;
let brief_owners = get_max_brief_owners::<T>(currency_id);
let caller: T::AccountId = brief_owners[0].clone();
let applicant: T::AccountId = create_account_id::<T>("applicant", 1);
let applicant: T::AccountId = create_account_id::<T>("applicant", 1, currency_id);
let budget = 10_000_000_000_000u128.saturated_into();
let initial_contribution = 5_000_000_000_000u128.saturated_into();
let brief_id = gen_hash(1);
Expand All @@ -120,8 +128,9 @@ mod benchmarks {
budget,
initial_contribution,
brief_id,
CurrencyId::Native,
currency_id,
milestones,
None,
false,
));
// (origin, brief_id)
Expand All @@ -137,11 +146,15 @@ mod benchmarks {
);
}

fn create_account_id<T: Config>(suri: &'static str, n: u32) -> T::AccountId {
fn create_account_id<T: Config>(
suri: &'static str,
n: u32,
currency_id: CurrencyId,
) -> T::AccountId {
let user = account(suri, n, SEED);
let initial_balance = 1_000_000_000_000_000u128;
assert_ok!(T::RMultiCurrency::deposit(
CurrencyId::Native,
currency_id,
&user,
initial_balance.saturated_into()
));
Expand All @@ -156,21 +169,21 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
assert_eq!(event, &system_event);
}

fn get_brief_owners<T: Config>(mut n: u32) -> BoundedBriefOwners<T> {
fn get_brief_owners<T: Config>(mut n: u32, currency: CurrencyId) -> BoundedBriefOwners<T> {
let max = <T as Config>::MaxBriefOwners::get();
if n > max {
n = max;
}
(0..n)
.map(|i| create_account_id::<T>("brief_owner", i))
.map(|i| create_account_id::<T>("brief_owner", i, currency))
.collect::<Vec<T::AccountId>>()
.try_into()
.expect("qed")
}

fn get_max_brief_owners<T: Config>() -> BoundedBriefOwners<T> {
fn get_max_brief_owners<T: Config>(currency_id: CurrencyId) -> BoundedBriefOwners<T> {
let max_brief_owners: u32 = <T as Config>::MaxBriefOwners::get();
get_brief_owners::<T>(max_brief_owners)
get_brief_owners::<T>(max_brief_owners, currency_id)
}

fn get_milestones<T: Config>(mut n: u32) -> BoundedProposedMilestones<T> {
Expand Down
1 change: 1 addition & 0 deletions pallets/briefs/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ fn create_proposal_from_brief() {
brief_id,
CurrencyId::Native,
get_milestones(10),
None,
false,
);

Expand Down
29 changes: 27 additions & 2 deletions pallets/briefs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ pub use pallet::*;
pub mod weights;
pub use weights::*;

pub mod migrations;

#[cfg(test)]
mod mock;

Expand Down Expand Up @@ -58,12 +60,12 @@ pub mod pallet {
BalanceOf<T>,
AccountIdOf<T>,
>>::StorageItem;
type DepositIdOf<T> =
pub(crate) type DepositIdOf<T> =
<<T as Config>::DepositHandler as DepositHandler<BalanceOf<T>, AccountIdOf<T>>>::DepositId;

pub type BriefHash = H256;

const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(3);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
Expand Down Expand Up @@ -156,6 +158,10 @@ pub mod pallet {
MilestonesTotalPercentageMustEqual100,
/// too many milestones here mate fixed with https://github.com/ImbueNetwork/imbue/issues/267
TooManyMilestones,
/// If youre using a foreign currency then you need an external_owned_address.
EoaRequiredForForeignCurrencies,
/// Currency is not supported for this external address.
CurrencyAccountComboNotSupported,
}

#[pallet::call]
Expand All @@ -174,6 +180,7 @@ pub mod pallet {
brief_id: BriefHash,
currency_id: CurrencyId,
milestones: BoundedProposedMilestones<T>,
external_owned_address: Option<common_types::ForeignOwnedAccount>,
require_fellowship: bool,
) -> DispatchResult {
let who = ensure_signed(origin)?;
Expand All @@ -187,6 +194,19 @@ pub mod pallet {
Error::<T>::BriefAlreadyExists
);

if let CurrencyId::ForeignAsset(_) = currency_id {
ensure!(
external_owned_address.is_some(),
Error::<T>::EoaRequiredForForeignCurrencies
);
}
if let Some(eoa) = external_owned_address {
ensure!(
eoa.ensure_supported_currency(currency_id),
Error::<T>::CurrencyAccountComboNotSupported
);
}

let total_percentage = milestones
.iter()
.fold(Percent::zero(), |acc: Percent, ms: &ProposedMilestone| {
Expand Down Expand Up @@ -239,6 +259,7 @@ pub mod pallet {
applicant,
milestones,
deposit_id,
external_owned_address,
);

Briefs::<T>::insert(brief_id, brief);
Expand Down Expand Up @@ -334,6 +355,7 @@ pub mod pallet {
.try_into()
.map_err(|_| Error::<T>::TooManyMilestones)?,
FundingPath::TakeFromReserved,
brief.eoa,
)?;

BriefContributions::<T>::remove(brief_id);
Expand Down Expand Up @@ -380,6 +402,7 @@ pub mod pallet {
pub applicant: AccountIdOf<T>,
pub milestones: BoundedProposedMilestones<T>,
pub deposit_id: DepositIdOf<T>,
pub eoa: Option<common_types::ForeignOwnedAccount>,
}

impl<T: Config> Pallet<T> {
Expand All @@ -406,6 +429,7 @@ pub mod pallet {
applicant: AccountIdOf<T>,
milestones: BoundedProposedMilestones<T>,
deposit_id: DepositIdOf<T>,
eoa: Option<common_types::ForeignOwnedAccount>,
) -> Self {
Self {
created_at,
Expand All @@ -415,6 +439,7 @@ pub mod pallet {
applicant,
milestones,
deposit_id,
eoa,
}
}
}
Expand Down
Loading

0 comments on commit 7e99ad5

Please sign in to comment.