Skip to content

Commit

Permalink
Benchmarking workflow (#290)
Browse files Browse the repository at this point in the history
* staging check for prs to main

* remove run gcp for push to staging

* Optional fellowship switch (#280)

* add test for approved applicant

* fmt

* add ensure role to runtime

---------

Co-authored-by: Sam Elamin <hussam.elamin@gmail.com>

* Foreign token minting tests (#282)

* test

* forign signer can mint

* negative minting test

* remove it (#285)

* Autofinalise dispute where jury size == 1 (#279)

* autocomplete dispute where jury size = 1

* refactor tests to include jury

* fmt

* give briefs a jury!

* fmt

* fix tests from merge

* remove it (#285)

---------

Co-authored-by: Sam Elamin <hussam.elamin@gmail.com>

* new benchmarks template

* remove old script

* benchmark script :)

* hmm

* clear votes with tests (#289)

* 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>

* add default fellows to selector, fix template

* fiddle

* generate new weights using script

* fmt

* use release profile

---------

Co-authored-by: Sam Elamin <hussam.elamin@gmail.com>
  • Loading branch information
f-gate and samelamin authored Dec 11, 2023
1 parent 86225b7 commit 6000339
Show file tree
Hide file tree
Showing 37 changed files with 1,297 additions and 471 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ on:
- "**.md"
env:
CARGO_TERM_COLOR: always
GCP_ZONE: europe-west3-a

jobs:
check_branch:
Expand All @@ -41,7 +40,6 @@ jobs:
image_family: ubuntu-2004-lts
machine_type: e2-highcpu-32
disk_size: 100
machine_zone: ${{ env.GCP_ZONE }}
ephemeral: true

test-features:
Expand Down
7 changes: 0 additions & 7 deletions ci/jobs/build-and-test.sh

This file was deleted.

7 changes: 0 additions & 7 deletions ci/jobs/clippy.sh

This file was deleted.

5 changes: 4 additions & 1 deletion libs/common-types/src/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ impl ForeignOwnedAccount {
}
#[cfg(feature = "runtime-benchmarks")]
pub fn get_supported_currency_eoa_combo() -> (ForeignOwnedAccount, CurrencyId) {
(ForeignOwnedAccount::ETH(Default::default()), CurrencyId::ForeignAsset(ForeignAssetId::ETH))
(
ForeignOwnedAccount::ETH(Default::default()),
CurrencyId::ForeignAsset(ForeignAssetId::ETH),
)
}
}

Expand Down
13 changes: 10 additions & 3 deletions pallets/briefs/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ mod benchmarks {
brief_id,
currency_id,
milestones,
Some(eoa)
Some(eoa),
false,
);
assert_last_event::<T>(Event::<T>::BriefSubmitted(caller, brief_id).into());
}
Expand All @@ -68,6 +69,7 @@ mod benchmarks {
currency_id,
milestones,
None,
false,
));
let brief_owner: T::AccountId = brief_owners[0].clone();
// (brief_owner, brief_id, contribution)
Expand All @@ -91,7 +93,6 @@ mod benchmarks {
let brief_id = gen_hash(1);
let milestones = get_max_milestones::<T>();


assert_ok!(Briefs::<T>::create_brief(
RawOrigin::Signed(caller).into(),
brief_owners,
Expand All @@ -102,6 +103,7 @@ mod benchmarks {
currency_id,
milestones,
None,
false,
));
// (origin, brief_id)
#[extrinsic_call]
Expand Down Expand Up @@ -129,6 +131,7 @@ mod benchmarks {
currency_id,
milestones,
None,
false,
));
// (origin, brief_id)
#[extrinsic_call]
Expand All @@ -143,7 +146,11 @@ mod benchmarks {
);
}

fn create_account_id<T: Config>(suri: &'static str, n: u32, currency_id: CurrencyId) -> 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(
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 @@ -23,6 +23,7 @@ fn create_proposal_from_brief() {
CurrencyId::Native,
get_milestones(10),
None,
false,
);

assert_ok!(BriefsMod::commence_work(
Expand Down
9 changes: 9 additions & 0 deletions pallets/briefs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub mod pallet {
use frame_system::pallet_prelude::*;
use orml_traits::{MultiCurrency, MultiReservableCurrency};
use pallet_deposits::traits::DepositHandler;
use pallet_fellowship::traits::EnsureRole;
use pallet_fellowship::traits::SelectJury;
use pallet_proposals::traits::IntoProposal;
use pallet_proposals::{Contribution, FundingPath, ProposedMilestone};
Expand Down Expand Up @@ -84,9 +85,12 @@ pub mod pallet {
type MaxMilestonesPerBrief: Get<u32>;
/// Storage deposits.
type BriefStorageItem: Get<StorageItemOf<Self>>;
/// Handler for deposits.
type DepositHandler: DepositHandler<BalanceOf<Self>, AccountIdOf<Self>>;
/// The type that selects a list of jury members.
type JurySelector: SelectJury<AccountIdOf<Self>>;
/// Type for ensuring an account is of a given fellowship role.
type EnsureRole: pallet_fellowship::traits::EnsureRole<AccountIdOf<Self>>;
/// The weight info for the extrinsics.
type WeightInfo: WeightInfoT;
}
Expand Down Expand Up @@ -177,9 +181,14 @@ pub mod pallet {
currency_id: CurrencyId,
milestones: BoundedProposedMilestones<T>,
external_owned_address: Option<common_types::ForeignOwnedAccount>,
require_fellowship: bool,
) -> DispatchResult {
let who = ensure_signed(origin)?;

if require_fellowship {
T::EnsureRole::ensure_role(&applicant, pallet_fellowship::Role::Freelancer, None)?;
}

ensure!(
Briefs::<T>::get(brief_id).is_none(),
Error::<T>::BriefAlreadyExists
Expand Down
1 change: 0 additions & 1 deletion pallets/briefs/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ pub(crate) mod v1 {
};

v2::BriefsV2::<T>::insert(key, migrated);

}
})
}
Expand Down
36 changes: 34 additions & 2 deletions pallets/briefs/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use sp_runtime::{
};

use pallet_deposits::traits::DepositHandler;
use pallet_fellowship::traits::FellowshipHandle;
use pallet_fellowship::Role;
use sp_std::{
convert::{TryFrom, TryInto},
str,
Expand Down Expand Up @@ -57,6 +59,7 @@ frame_support::construct_runtime!(
BriefsMod: pallet_briefs::{Pallet, Call, Storage, Event<T>},
Proposals: pallet_proposals::{Pallet, Call, Storage, Event<T>},
Identity: pallet_identity::{Pallet, Call, Storage, Event<T>},
Fellowship: pallet_fellowship::{Pallet, Call, Storage, Event<T>},
}
);

Expand Down Expand Up @@ -209,6 +212,7 @@ impl pallet_briefs::Config for Test {
type DepositHandler = MockDepositHandler;
type WeightInfo = pallet_briefs::WeightInfo<Self>;
type JurySelector = MockJurySelector;
type EnsureRole = pallet_fellowship::impls::EnsureFellowshipRole<Self>;
}

parameter_types! {
Expand Down Expand Up @@ -273,14 +277,39 @@ impl pallet_identity::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub MaxCandidatesPerShortlist: u32 = 100;
pub ShortlistPeriod: BlockNumber = 100;
pub MembershipDeposit: Balance = 50_000_000;
pub SlashAccount: AccountId = 1;
pub DepositCurrencyId: CurrencyId = CurrencyId::Native;
}

impl pallet_fellowship::Config for Test {
type RuntimeEvent = RuntimeEvent;
type MultiCurrency = Tokens;
type ForceAuthority = EnsureRoot<AccountId>;
type MaxCandidatesPerShortlist = MaxCandidatesPerShortlist;
type ShortlistPeriod = ShortlistPeriod;
type MembershipDeposit = MembershipDeposit;
type DepositCurrencyId = DepositCurrencyId;
type SlashAccount = SlashAccount;
type Permissions = pallet_fellowship::impls::VetterAndFreelancerAllPermissions;
type WeightInfo = pallet_fellowship::weights::WeightInfo<Test>;
}

parameter_types! {
pub const UnitWeightCost: u64 = 10;
pub const MaxInstructions: u32 = 100;
}

pub static ALICE: AccountId = 125;
pub static BOB: AccountId = 126;
pub static CHARLIE: AccountId = 127;
pub static FREELANCER: AccountId = 1270;
pub static TREASURY: AccountId = 200;
pub static JURY_1: AccountId = 1002;
pub static JURY_2: AccountId = 1001;

pub(crate) fn build_test_externality() -> sp_io::TestExternalities {
let mut t = frame_system::GenesisConfig::<Test>::default()
Expand All @@ -292,7 +321,7 @@ pub(crate) fn build_test_externality() -> sp_io::TestExternalities {
.unwrap();
orml_tokens::GenesisConfig::<Test> {
balances: {
vec![ALICE, BOB, CHARLIE]
vec![ALICE, BOB, CHARLIE, FREELANCER]
.into_iter()
.map(|id| (id, CurrencyId::Native, 1000000))
.collect::<Vec<_>>()
Expand All @@ -303,6 +332,7 @@ pub(crate) fn build_test_externality() -> sp_io::TestExternalities {

let mut ext = sp_io::TestExternalities::new(t);
ext.execute_with(|| {
pallet_fellowship::Roles::<Test>::insert(&FREELANCER, (Role::Freelancer, 10));
System::set_block_number(1);
});
ext
Expand All @@ -312,7 +342,9 @@ pub struct MockJurySelector;
impl pallet_fellowship::traits::SelectJury<AccountId> for MockJurySelector {
type JurySize = MaxJuryMembers;
fn select_jury() -> BoundedVec<AccountId, Self::JurySize> {
BoundedVec::new()
vec![JURY_1, JURY_2]
.try_into()
.expect("should be below bound.")
}
}

Expand Down
55 changes: 53 additions & 2 deletions pallets/briefs/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,55 @@ use crate::*;
use common_types::CurrencyId;
use frame_support::{assert_noop, assert_ok, pallet_prelude::*};
use orml_traits::{MultiCurrency, MultiReservableCurrency};
use pallet_fellowship::traits::EnsureRole;
use pallet_proposals::{BoundedProposedMilestones, Projects, ProposedMilestone};
use sp_arithmetic::per_things::Percent;
use sp_runtime::DispatchError::BadOrigin;

use std::convert::TryInto;

#[test]
fn create_brief_not_approved_applicant() {
build_test_externality().execute_with(|| {
// TODO:
// Only accounts in the fellowship can apply for work
assert_noop!(
BriefsMod::create_brief(
RuntimeOrigin::signed(BOB),
get_brief_owners(u32::MAX),
ALICE,
100000,
10000,
gen_hash(1),
CurrencyId::Native,
get_milestones(10),
None,
true,
),
BadOrigin
);
});
}

#[test]
fn create_brief_approved_applicant() {
build_test_externality().execute_with(|| {
assert_ok!(<Test as Config>::EnsureRole::ensure_role(
&FREELANCER,
pallet_fellowship::Role::Freelancer,
None
));

assert_ok!(BriefsMod::create_brief(
RuntimeOrigin::signed(BOB),
get_brief_owners(10),
FREELANCER,
100000,
10000,
gen_hash(1),
CurrencyId::Native,
get_milestones(10),
None,
true,
));
});
}

Expand All @@ -33,6 +72,7 @@ fn create_brief_brief_owner_overflow() {
CurrencyId::Native,
get_milestones(10),
None,
false,
),
Error::<Test>::TooManyBriefOwners
);
Expand All @@ -52,6 +92,7 @@ fn create_brief_with_no_contribution_ok() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));
});
}
Expand All @@ -73,6 +114,7 @@ fn create_brief_no_contribution_and_contribute() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

(0..5).for_each(|_| {
Expand Down Expand Up @@ -115,6 +157,7 @@ fn contribute_to_brief_not_brief_owner() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_noop!(
Expand Down Expand Up @@ -144,6 +187,7 @@ fn contribute_to_brief_more_than_total_ok() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));
assert_ok!(BriefsMod::contribute_to_brief(
RuntimeOrigin::signed(BOB),
Expand All @@ -169,6 +213,7 @@ fn create_brief_already_exists() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_noop!(
Expand All @@ -182,6 +227,7 @@ fn create_brief_already_exists() {
CurrencyId::Native,
get_milestones(10),
None,
false,
),
Error::<Test>::BriefAlreadyExists
);
Expand All @@ -204,6 +250,7 @@ fn only_applicant_can_start_work() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_noop!(
Expand Down Expand Up @@ -234,6 +281,7 @@ fn initial_contribution_and_extra_contribution_aggregates() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_ok!(BriefsMod::contribute_to_brief(
Expand Down Expand Up @@ -271,6 +319,7 @@ fn reserved_funds_are_transferred_to_project_kitty() {
CurrencyId::Native,
get_milestones(10),
None,
false,
);

assert_ok!(BriefsMod::commence_work(
Expand Down Expand Up @@ -304,6 +353,7 @@ fn cancel_brief_works() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_ok!(BriefsMod::contribute_to_brief(
Expand Down Expand Up @@ -371,6 +421,7 @@ fn cancel_brief_not_brief_owner() {
CurrencyId::Native,
get_milestones(10),
None,
false,
));

assert_noop!(
Expand Down
Loading

0 comments on commit 6000339

Please sign in to comment.