From 7fe42a2364f1a74e237945a0478eee2417fab155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20Eng=C3=A9libert?= <tuxmain@zettascript.org> Date: Tue, 24 Sep 2024 18:16:19 +0200 Subject: [PATCH] Fix distance end2end tests (nodes/rust/duniter-v2s!276) * cucumber: distance_fail treasury * distance: handle slash unbalance * end2endtests: fail on skipped * Fix distance end2end tests * Make it explicit that distance slashes from reserve --- end2end-tests/README.md | 15 +- .../cucumber-features/distance_fail.feature | 50 ++++++ .../identity_creation.feature | 12 +- .../cucumber-genesis/bad_distance.json | 162 ++++++++++++++++++ end2end-tests/tests/common/distance.rs | 17 +- end2end-tests/tests/cucumber_tests.rs | 78 +++------ pallets/distance/src/lib.rs | 12 +- pallets/distance/src/mock.rs | 1 + pallets/membership/src/lib.rs | 2 +- runtime/common/src/pallets_config.rs | 1 + 10 files changed, 286 insertions(+), 64 deletions(-) create mode 100644 end2end-tests/cucumber-features/distance_fail.feature create mode 100644 end2end-tests/cucumber-genesis/bad_distance.json diff --git a/end2end-tests/README.md b/end2end-tests/README.md index 5e298eb75..1205be0da 100644 --- a/end2end-tests/README.md +++ b/end2end-tests/README.md @@ -125,7 +125,7 @@ For some scenarios, you may need to perform an action (When) that fails voluntar ### Run cucumber functional tests -The cucumber tests use the last debug binary in your `target` folder. Make sure this binary corresponds to the executable you want to test by running `cargo build` before. +The cucumber tests use the last debug binary in your `target` folder. Make sure this binary corresponds to the executable you want to test by running `cargo build --features constant-fees` before. To run the cucumber tests, you will need to have the rust toolchain installed locally. @@ -168,6 +168,19 @@ subxt metadata -f bytes --version 14 > resources/metadata.scale If you don't have subxt, install it: `cargo install subxt-cli` +### Debug + +Cucumber uses carriage returns to pretty-print. +Hence you may need to append an additional newline to your debug printings. + +This commandline can be used to enable more debugging: + +```bash +RUST_LOG=debug cargo cucumber -i distance_fail.feature -vvv --show-output +``` + +You can use the `log::debug` macro if the line `env_logger::init()` is uncommented in `cucumber_tests.rs`. + [BDD]: https://en.wikipedia.org/wiki/Behavior-driven_development [cucumber]: https://cucumber.io/ [Cucumber Rust Book]: https://cucumber-rs.github.io/cucumber/current/writing/index.html diff --git a/end2end-tests/cucumber-features/distance_fail.feature b/end2end-tests/cucumber-features/distance_fail.feature new file mode 100644 index 000000000..1abb9df53 --- /dev/null +++ b/end2end-tests/cucumber-features/distance_fail.feature @@ -0,0 +1,50 @@ +@genesis.bad_distance + +Feature: Distance fail +# +# WoT: +# +# H<->G<->E<->D<->C-->B +# ^ ^ ^ ^ +# \ / \ / +# v v v v +# I A +# +# Every member is referee. Referee count = 8; 80% = 6.4 +# Certs from Alice and Bob do not ensure the fulfilling of the distance rule +# because the newcomer would reach only 6 members up to G. + + + Scenario: an unvalidated member fails the distance rule + Then treasury should contain 1 ÄžD + When alice sends 7 ÄžD to ferdie + Then alice should have 0 ÄžD reserved + Then alice should have 199 cÄžD + When bob sends 750 cÄžD to ferdie + When 15 block later + When alice creates identity for ferdie + Then ferdie identity should be unconfirmed + Then ferdie should be certified by alice + When ferdie confirms his identity with pseudo "ferdie" + Then ferdie identity should be unvalidated + When 3 block later + When bob certifies ferdie + Then ferdie should be certified by bob + Then ferdie should have 0 ÄžD reserved + Then ferdie should have 1449 cÄžD + When ferdie requests distance evaluation + Then ferdie should have 10 ÄžD reserved + Then ferdie should have 449 cÄžD + When 7 blocks later + Then treasury should contain 102 cÄžD + When alice runs distance oracle + When 7 blocks later + Then ferdie should be certified by alice + Then ferdie should be certified by bob + # The distance rule is failed + Then ferdie identity should be unvalidated + # Ferdie got his reserve slashed + Then ferdie should have 0 ÄžD reserved + Then ferdie should have 449 cÄžD + # Slashed amount is transfered to treasury + Then treasury should contain 1102 cÄžD diff --git a/end2end-tests/cucumber-features/identity_creation.feature b/end2end-tests/cucumber-features/identity_creation.feature index b4d321ddd..97ae14b02 100644 --- a/end2end-tests/cucumber-features/identity_creation.feature +++ b/end2end-tests/cucumber-features/identity_creation.feature @@ -2,11 +2,11 @@ Feature: Identity creation Scenario: alice invites a new member to join the web of trust # 6 ÄžD covers: - # - account creation fees (3 ÄžD) # - existential deposit (1 ÄžD) # - transaction fees (below 1 ÄžD) When alice sends 7 ÄžD to dave # Alice is treasury funder for 1 ÄžD => 10-1-7 = 2 (minus TODO fees) + Then alice should have 0 ÄžD reserved Then alice should have 199 cÄžD When bob sends 750 cÄžD to dave When charlie sends 6 ÄžD to eve @@ -23,10 +23,14 @@ Feature: Identity creation When charlie certifies dave Then dave should be certified by bob Then dave should be certified by charlie + Then dave should have 0 ÄžD reserved + Then dave should have 1449 cÄžD When dave requests distance evaluation - Then dave should have distance result in 2 periods - When 14 blocks later - Then dave should have distance result in 1 period + Then dave should have 10 ÄžD reserved + Then dave should have 449 cÄžD + When 7 blocks later When alice runs distance oracle When 7 blocks later Then dave identity should be member + Then dave should have 0 ÄžD reserved + Then dave should have 1449 cÄžD diff --git a/end2end-tests/cucumber-genesis/bad_distance.json b/end2end-tests/cucumber-genesis/bad_distance.json new file mode 100644 index 000000000..cb040ba2c --- /dev/null +++ b/end2end-tests/cucumber-genesis/bad_distance.json @@ -0,0 +1,162 @@ +{ + "first_ud": null, + "first_ud_reeval": null, + "genesis_parameters": { + "genesis_certs_expire_on": 1000, + "genesis_certs_min_received": 2, + "genesis_memberships_expire_on": 1000, + "genesis_smith_certs_expire_on": 1000, + "genesis_smith_certs_min_received": 2, + "genesis_smith_memberships_expire_on": 100000 + }, + "identities": { + "Alice": { + "index": 1, + "balance": 1000, + "certs_received": { + "Bob": 2700000000, + "Charlie": 2700000000 + }, + "owner_address": "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Bob": { + "index": 2, + "balance": 1000, + "certs_received": { + "Alice": 2700000000, + "Charlie": 2700000000 + }, + "owner_address": "5FHneW46xGXgs5mUiveU4sbTyGBzmstUspZC92UhjJM694ty", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Charlie": { + "index": 3, + "balance": 1000, + "certs_received": { + "Alice": 2700000000, + "Dave": 2700000000 + }, + "owner_address": "5FLSigC9HGRKVhB9FiEo4Y3koPsNmBmLJbpXg2mp1hXcS59Y", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Dave": { + "index": 4, + "balance": 1000, + "certs_received": { + "Charlie": 2700000000, + "Eve": 2700000000 + }, + "owner_address": "5EWtNo12S57725GF641a6avLkHaXMWnJ5RPoo721GR92gSEt", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Eve": { + "index": 5, + "balance": 1000, + "certs_received": { + "Dave": 2700000000, + "Gertrude": 2700000000 + }, + "owner_address": "5EjiXi8p1sCEUevSCQkHom6yUFQY9N5U5xpEofE2N4ZhJ9vs", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Gertrude": { + "index": 6, + "balance": 1000, + "certs_received": { + "Eve": 2700000000, + "Henry": 2700000000, + "Irene": 2700000000 + }, + "owner_address": "5Hgx76GUW5ETtxTX53BJqzeRtxTASC4AgCPwGDuUjvSuKbC6", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Henry": { + "index": 7, + "balance": 1000, + "certs_received": { + "Irene": 2700000000, + "Gertrude": 2700000000 + }, + "owner_address": "5DJZ3Ns49G7B8tr2av53WZorSsMLzvfhPPVeS2RPispMK3Y9", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + }, + "Irene": { + "index": 8, + "balance": 1000, + "certs_received": { + "Henry": 2700000000, + "Gertrude": 2700000000 + }, + "owner_address": "5EqdTcg58VmBfd7DGPj1GJeHahqUEh3fjWH2fS4KRQmhpAjN", + "membership_expire_on": 2700000000, + "membership_revokes_on": 2700000001, + "revoked": false, + "next_cert_issuable_on": 0 + } + }, + "parameters": { + "babe_epoch_duration": 30, + "cert_period": 15, + "cert_max_by_issuer": 10, + "cert_min_received_cert_to_issue_cert": 2, + "cert_validity_period": 1000, + "idty_confirm_period": 40, + "idty_creation_period": 50, + "membership_period": 1000, + "membership_renewal_period": 500, + "ud_creation_period": 60000, + "ud_reeval_period": 600000, + "smith_cert_max_by_issuer": 8, + "smith_inactivity_max_duration": 48, + "smith_wot_min_cert_for_membership": 2, + "wot_first_cert_issuable_on": 20, + "wot_min_cert_for_create_idty_right": 2, + "wot_min_cert_for_membership": 2 + }, + "clique_smiths": [ + { + "name": "Alice" + }, + { + "name": "Bob" + }, + { + "name": "Charlie" + } + ], + "sudo_key": "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY", + "technical_committee": [ + "Alice", + "Bob", + "Charlie" + ], + "treasury_funder_pubkey": "FHNpKmJrUtusuvKPGomAygQqeiks98bdV6yD61Stb6vg", + "ud": 1000, + "initial_monetary_mass": 8000, + "current_block": { + "number": 0, + "medianTime": 1700000000 + } +} diff --git a/end2end-tests/tests/common/distance.rs b/end2end-tests/tests/common/distance.rs index 29a3a12c9..f9f43d670 100644 --- a/end2end-tests/tests/common/distance.rs +++ b/end2end-tests/tests/common/distance.rs @@ -61,13 +61,13 @@ pub async fn run_oracle( ) .await { - for _ in 0..30 { + // Distance evaluation period is 7 blocks + for _ in 0..7 { super::create_empty_block(&client.rpc).await?; } - let _events = create_block_with_extrinsic( - &client.rpc, - client.client + &client.rpc, + client.client .tx() .create_signed( &gdev::tx().sudo().sudo(gdev::runtime_types::gdev_runtime::RuntimeCall::Distance( @@ -86,6 +86,15 @@ pub async fn run_oracle( .await?, ) .await?; + /*for event in events.iter() { + let event = event.unwrap(); + println!( + "Event: {}::{} -> {:?}\n\n", + event.pallet_name(), + event.variant_name(), + event.field_values() + ); + }*/ } Ok(()) diff --git a/end2end-tests/tests/cucumber_tests.rs b/end2end-tests/tests/cucumber_tests.rs index add3f14a0..642779ec8 100644 --- a/end2end-tests/tests/cucumber_tests.rs +++ b/end2end-tests/tests/cucumber_tests.rs @@ -369,12 +369,33 @@ async fn run_distance_oracle(world: &mut DuniterWorld, who: String) -> Result<() // ===== then ==== #[allow(clippy::needless_pass_by_ref_mut)] -#[then(regex = r"([a-zA-Z]+) should have (\d+) (ÄžD|cÄžD)")] +#[then(regex = r"treasury should contain (\d+) (ÄžD|cÄžD)")] +async fn treasury_should_contain( + world: &mut DuniterWorld, + amount: u64, + unit: String, +) -> Result<()> { + let who = + subxt::utils::AccountId32::from_str("5EYCAe5ijiYfyeZ2JJCGq56LmPyNRAKzpG4QkoQkkQNB5e6Z") + .expect("invalid treasury account id"); + let (amount, _is_ud) = parse_amount(amount, &unit); + + let who_account = world + .read_or_default(&gdev::storage().system().account(&who)) + .await + .await?; + assert_eq!(who_account.data.free, amount); + Ok(()) +} + +#[allow(clippy::needless_pass_by_ref_mut)] +#[then(regex = r"([a-zA-Z]+) should have (\d+) (ÄžD|cÄžD)( reserved)?")] async fn should_have( world: &mut DuniterWorld, who: String, amount: u64, unit: String, + reserved: String, ) -> Result<()> { // Parse inputs let who: subxt::utils::AccountId32 = AccountKeyring::from_str(&who) @@ -387,7 +408,11 @@ async fn should_have( .read_or_default(&gdev::storage().system().account(&who)) .await .await?; - assert_eq!(who_account.data.free, amount); + if reserved.is_empty() { + assert_eq!(who_account.data.free, amount); + } else { + assert_eq!(who_account.data.reserved, amount); + } Ok(()) } @@ -501,53 +526,6 @@ async fn should_be_certified_by( } } -#[allow(clippy::needless_pass_by_ref_mut)] -#[then(regex = r"([a-zA-Z]+) should have distance result in (\d+) sessions?")] -async fn should_have_distance_result_in_sessions( - world: &mut DuniterWorld, - who: String, - sessions: u32, -) -> Result<()> { - assert!(sessions < 3, "Session number must be < 3"); - - let who: subxt::utils::AccountId32 = AccountKeyring::from_str(&who) - .unwrap() - .to_account_id() - .into(); - - let idty_id = world - .read(&gdev::storage().identity().identity_index_of(who)) - .await - .await? - .unwrap(); - - let current_session = world - .read(&gdev::storage().session().current_index()) - .await - .await? - .unwrap_or_default(); - - let pool = world - .read(&match (current_session + sessions) % 3 { - 0 => gdev::storage().distance().evaluation_pool0(), - 1 => gdev::storage().distance().evaluation_pool1(), - 2 => gdev::storage().distance().evaluation_pool2(), - _ => unreachable!("n%3<3"), - }) - .await - .await - .unwrap() - .ok_or_else(|| anyhow::anyhow!("given pool is empty"))?; - - for (sample_idty, _) in pool.evaluations.0 { - if sample_idty == idty_id { - return Ok(()); - } - } - - Err(anyhow::anyhow!("no evaluation in given pool").into()) -} - use gdev::runtime_types::pallet_identity::types::IdtyStatus; // status from string @@ -628,7 +606,7 @@ async fn main() { .expect("Error setting Ctrl-C handler"); let summarize = DuniterWorld::cucumber() - //.fail_on_skipped() + .fail_on_skipped() .max_concurrent_scenarios(4) .before(move |feature, _rule, scenario, world| { let mut genesis_conf_file_path = PathBuf::new(); diff --git a/pallets/distance/src/lib.rs b/pallets/distance/src/lib.rs index 82997a880..ba5e80086 100644 --- a/pallets/distance/src/lib.rs +++ b/pallets/distance/src/lib.rs @@ -82,9 +82,9 @@ pub use types::*; pub use weights::WeightInfo; use frame_support::traits::{ - fungible::{self, hold::Balanced, Mutate, MutateHold}, + fungible::{self, hold, Credit, Mutate, MutateHold}, tokens::Precision, - StorageVersion, + OnUnbalanced, StorageVersion, }; use sp_distance::{InherentError, INHERENT_IDENTIFIER}; use sp_inherents::{InherentData, InherentIdentifier}; @@ -132,7 +132,7 @@ pub mod pallet { /// Currency type used in this pallet for reserve and slash operations. type Currency: Mutate<Self::AccountId> + MutateHold<Self::AccountId, Reason = Self::RuntimeHoldReason> - + Balanced<Self::AccountId>; + + hold::Balanced<Self::AccountId>; /// The overarching hold reason type. type RuntimeHoldReason: From<HoldReason>; @@ -155,6 +155,9 @@ pub mod pallet { #[pallet::constant] type MinAccessibleReferees: Get<Perbill>; + /// Handler for unbalanced reduction when invalid distance causes a slash. + type OnUnbalanced: OnUnbalanced<Credit<Self::AccountId, Self::Currency>>; + /// The overarching event type. type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>; @@ -668,11 +671,12 @@ pub mod pallet { ); } else { // Negative result, slash and deposit event - let _ = T::Currency::slash( + let (imbalance, _) = <T::Currency as hold::Balanced<_>>::slash( &HoldReason::DistanceHold.into(), &requester, <T as Config>::EvaluationPrice::get(), ); + T::OnUnbalanced::on_unbalanced(imbalance); Self::deposit_event(Event::EvaluatedInvalid { idty_index: idty, distance, diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs index f837bac85..fa963500b 100644 --- a/pallets/distance/src/mock.rs +++ b/pallets/distance/src/mock.rs @@ -261,6 +261,7 @@ impl pallet_distance::Config for Test { type EvaluationPrice = frame_support::traits::ConstU64<1000>; type MaxRefereeDistance = frame_support::traits::ConstU32<5>; type MinAccessibleReferees = MinAccessibleReferees; + type OnUnbalanced = (); type OnValidDistanceStatus = (); type RuntimeEvent = RuntimeEvent; type RuntimeHoldReason = RuntimeHoldReason; diff --git a/pallets/membership/src/lib.rs b/pallets/membership/src/lib.rs index b661c6273..683e5fe1c 100644 --- a/pallets/membership/src/lib.rs +++ b/pallets/membership/src/lib.rs @@ -246,7 +246,7 @@ pub mod pallet { Error::<T>::AlreadyMember ); - // enough certifications and distance rule for example + // check status and enough certifications T::CheckMembershipOpAllowed::check_add_membership(idty_id)?; Ok(()) } diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs index e82397cd9..3b5ae90ba 100644 --- a/runtime/common/src/pallets_config.rs +++ b/runtime/common/src/pallets_config.rs @@ -519,6 +519,7 @@ macro_rules! pallets_config { type EvaluationPrice = frame_support::traits::ConstU64<1000>; type MaxRefereeDistance = MaxRefereeDistance; type MinAccessibleReferees = MinAccessibleReferees; + type OnUnbalanced = HandleFees; type OnValidDistanceStatus = Wot; type RuntimeEvent = RuntimeEvent; type RuntimeHoldReason = RuntimeHoldReason; -- GitLab