Fix identities live tests (!146)

* update live tests * wip add position of duplicate * wip add counter for owner key * wip add coherence test * wip

Fix identities live tests (!146)
* update live tests * wip add position of duplicate * wip add counter for owner key * wip add coherence test * wip
cca4eb30 · Hugo Trentesaux · 335ab2cc · cca4eb30 · cca4eb30 · cca4eb30
Commit cca4eb30 authored 1 year ago by Hugo Trentesaux
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -210,7 +210,7 @@ version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cab112f0a86d568ea0e627cc1d6be74a1e9cd55214684db5561995f6dad897c6"
 dependencies = [
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -259,7 +259,7 @@ dependencies = [
 "asn1-rs-impl",
 "displaydoc",
 "nom",
- "num-traits",
+ "num-traits 0.2.15",
 "rusticata-macros",
 "thiserror",
 "time 0.3.23",
@@ -275,7 +275,7 @@ dependencies = [
 "asn1-rs-impl",
 "displaydoc",
 "nom",
- "num-traits",
+ "num-traits 0.2.15",
 "rusticata-macros",
 "thiserror",
 "time 0.3.23",
@@ -826,7 +826,7 @@ dependencies = [
 "iana-time-zone",
 "js-sys",
 "num-integer",
- "num-traits",
+ "num-traits 0.2.15",
 "time 0.1.45",
 "wasm-bindgen",
 "winapi 0.3.9",
@@ -1145,6 +1145,15 @@ dependencies = [
 "memchr",
 ]

+[[package]]
+name = "countmap"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1ef2a403c4af585607826502480ab6e453f320c230ef67255eee21f0cc72c0a6"
+dependencies = [
+ "num-traits 0.1.43",
+]
+
 [[package]]
 name = "cpp_demangle"
 version = "0.3.5"
@@ -1688,7 +1697,7 @@ dependencies = [
 "displaydoc",
 "nom",
 "num-bigint",
- "num-traits",
+ "num-traits 0.2.15",
 "rusticata-macros",
 ]

@@ -1702,7 +1711,7 @@ dependencies = [
 "displaydoc",
 "nom",
 "num-bigint",
- "num-traits",
+ "num-traits 0.2.15",
 "rusticata-macros",
 ]

@@ -1868,7 +1877,7 @@ dependencies = [
 "flate2",
 "fnv",
 "log",
- "num-traits",
+ "num-traits 0.2.15",
 "parity-scale-codec",
 "rayon",
 "simple_logger",
@@ -1914,7 +1923,7 @@ dependencies = [

 [[package]]
 name = "duniter"
-version = "0.3.0"
+version = "0.7.0"
 dependencies = [
 "async-io",
 "bs58 0.5.0",
@@ -2023,6 +2032,7 @@ name = "duniter-live-tests"
 version = "3.0.0"
 dependencies = [
 "anyhow",
+ "countmap",
 "hex-literal",
 "parity-scale-codec",
 "sp-core",
@@ -2423,7 +2433,7 @@ dependencies = [
 "futures 0.3.29",
 "futures-timer",
 "log",
- "num-traits",
+ "num-traits 0.2.15",
 "parity-scale-codec",
 "parking_lot 0.12.1",
 "scale-info",
@@ -2464,7 +2474,7 @@ version = "0.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4"
 dependencies = [
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -3889,7 +3899,7 @@ version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "276ec31bcb4a9ee45f58bec6f9ec700ae4cf4f4f8f2fa7e06cb406bd5ffdd770"
 dependencies = [
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -5194,7 +5204,7 @@ dependencies = [
 "nalgebra-macros",
 "num-complex",
 "num-rational",
- "num-traits",
+ "num-traits 0.2.15",
 "simba",
 "typenum",
 ]
@@ -5403,7 +5413,7 @@ checksum = "f93ab6289c7b344a8a9f60f88d80aa20032336fe78da341afc91c8a2341fc75f"
 dependencies = [
 "autocfg",
 "num-integer",
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -5412,7 +5422,7 @@ version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "02e0d21255c828d6f128a1e41534206671e8c3ea0c62f32291e808dc82cff17d"
 dependencies = [
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -5432,7 +5442,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "225d3389fb3509a24c93f5c29eb6bde2586b98d9f016636dff58d7c6f7569cd9"
 dependencies = [
 "autocfg",
- "num-traits",
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -5444,7 +5454,16 @@ dependencies = [
 "autocfg",
 "num-bigint",
 "num-integer",
- "num-traits",
+ "num-traits 0.2.15",
+]
+
+[[package]]
+name = "num-traits"
+version = "0.1.43"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "92e5113e9fd4cc14ded8e499429f396a20f98c772a47cc8622a736e1ec843c31"
+dependencies = [
+ "num-traits 0.2.15",
 ]

 [[package]]
@@ -7724,7 +7743,7 @@ dependencies = [
 "log",
 "num-bigint",
 "num-rational",
- "num-traits",
+ "num-traits 0.2.15",
 "parity-scale-codec",
 "parking_lot 0.12.1",
 "sc-client-api",
@@ -8494,7 +8513,7 @@ dependencies = [
 "futures-timer",
 "linked-hash-map",
 "log",
- "num-traits",
+ "num-traits 0.2.15",
 "parity-scale-codec",
 "parking_lot 0.12.1",
 "sc-client-api",
@@ -9038,7 +9057,7 @@ checksum = "061507c94fc6ab4ba1c9a0305018408e312e17c041eb63bef8aa726fa33aceae"
 dependencies = [
 "approx",
 "num-complex",
- "num-traits",
+ "num-traits 0.2.15",
 "paste",
 "wide",
 ]
@@ -9195,7 +9214,7 @@ version = "6.0.0"
 source = "git+https://github.com/duniter/substrate?branch=duniter-substrate-v0.9.42#38b19717f847d3eda654b6465802c244ea6372a6"
 dependencies = [
 "integer-sqrt",
- "num-traits",
+ "num-traits 0.2.15",
 "parity-scale-codec",
 "scale-info",
 "serde",
@@ -11280,7 +11299,7 @@ dependencies = [
 "libm",
 "memory_units",
 "num-rational",
- "num-traits",
+ "num-traits 0.2.15",
 "region",
 ]


--- a/live-tests/Cargo.toml
+++ b/live-tests/Cargo.toml
@@ -15,4 +15,5 @@ parity-scale-codec = "3.4.0"
 sp-core = { git = 'https://github.com/duniter/substrate', branch = 'duniter-substrate-v0.9.42', default-features = false }
 subxt = { git = 'https://github.com/duniter/subxt', branch = 'duniter-substrate-v0.9.42', default-features = false, features = ["jsonrpsee-ws"] }
 tokio = { version = "1.28", features = ["macros", "time", "rt-multi-thread"], default-features = false }
-sp-runtime = {  git = 'https://github.com/duniter/substrate', branch = 'duniter-substrate-v0.9.42', default-features = false , features = ["std"] } # https://github.com/paritytech/subxt/issues/437
\ No newline at end of file
+sp-runtime = {  git = 'https://github.com/duniter/substrate', branch = 'duniter-substrate-v0.9.42', default-features = false , features = ["std"] } # https://github.com/paritytech/subxt/issues/437
+countmap = "0.2.0"
--- a/live-tests/README.md
+++ b/live-tests/README.md
@@ -16,7 +16,7 @@ Test suite that verifies the consistency of the onchain storage.
 #### Custom RPC endpoint

 You can choose to use another RPC endpoint by setting the environment variable `WS_RPC_ENDPOINT`.
-This is also the only way to test against a different network that the default one.
+This is also the only way to test against a different network that the default one which is `ws://localhost:9944`.

 #### run against a specific block


--- a/live-tests/tests/sanity_gdev.rs
+++ b/live-tests/tests/sanity_gdev.rs
@@ -17,14 +17,16 @@
 #[subxt::subxt(runtime_metadata_path = "../resources/metadata.scale")]
 pub mod gdev {}

+use countmap::CountMap;
 use hex_literal::hex;
 use sp_core::crypto::AccountId32;
 use sp_core::{blake2_128, ByteArray, H256};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use subxt::config::SubstrateConfig as GdevConfig;

-const DEFAULT_ENDPOINT: &str = "wss://gdev.librelois.fr:443/ws";
+const DEFAULT_ENDPOINT: &str = "ws://localhost:9944";

+const EXISTENTIAL_DEPOSIT: u64 = 100;
 const TREASURY_ACCOUNT_ID: [u8; 32] =
    hex!("6d6f646c70792f74727372790000000000000000000000000000000000000000");

@@ -89,7 +91,7 @@ async fn sanity_tests_at(client: Client, _maybe_block_hash: Option<H256>) -> any
        account_id_bytes.copy_from_slice(&key.0[48..]);
        accounts.insert(AccountId32::new(account_id_bytes), account_info);
    }
-    println!("accounts: {}.", accounts.len());
+    println!("accounts.len(): {}.", accounts.len());

    // Collect identities
    let mut identities: HashMap<IdtyIndex, IdtyValue> = HashMap::new();
@@ -113,7 +115,7 @@ async fn sanity_tests_at(client: Client, _maybe_block_hash: Option<H256>) -> any
        };
        identities.insert(IdtyIndex::from_le_bytes(idty_index_bytes), idty_val);
    }
-    println!("identities: {}.", identities.len());
+    println!("identities.len(): {}.", identities.len());

    // Collect identity_index_of
    let mut identity_index_of: HashMap<[u8; 16], IdtyIndex> = HashMap::new();
@@ -126,10 +128,10 @@ async fn sanity_tests_at(client: Client, _maybe_block_hash: Option<H256>) -> any
        .await?;
    while let Some((key, idty_index)) = idty_index_of_iter.next().await? {
        let mut blake2_128_bytes = [0u8; 16];
-        blake2_128_bytes.copy_from_slice(&key.0[32..]);
+        blake2_128_bytes.copy_from_slice(&key.0[32..48]);
        identity_index_of.insert(blake2_128_bytes, idty_index);
    }
-    println!("identity_index_of: {}.", identities.len());
+    println!("identity_index_of.len(): {}.", identity_index_of.len());

    let storage = Storage {
        accounts,
@@ -154,12 +156,17 @@ mod verifier {
            Self { errors: Vec::new() }
        }

+        // FIXME why async functions when called with await?
+
+        /// method to run all storage tests
        pub(super) async fn verify_storage(&mut self, storage: &Storage) -> anyhow::Result<()> {
            self.verify_accounts(&storage.accounts).await;
            self.verify_identities(&storage.accounts, &storage.identities)
                .await;
            self.verify_identity_index_of(&storage.identities, &storage.identity_index_of)
                .await;
+            self.verify_identity_coherence(&storage.identities, &storage.identity_index_of)
+                .await;

            if self.errors.is_empty() {
                Ok(())
@@ -174,12 +181,19 @@ mod verifier {
            }
        }

+        /// assert method to collect errors
        fn assert(&mut self, assertion: bool, error: String) {
            if !assertion {
                self.errors.push(error);
            }
        }

+        /// like assert but just push error
+        fn error(&mut self, error: String) {
+            self.errors.push(error);
+        }
+
+        /// check accounts sufficients and consumers (specific to duniter-account pallet)
        async fn verify_accounts(&mut self, accounts: &HashMap<AccountId32, AccountInfo>) {
            for (account_id, account_info) in accounts {
                if account_info.sufficients == 0 {
@@ -190,7 +204,8 @@ mod verifier {
                    );
                    // Rule 2: If the account is not sufficient, it should comply to the existential deposit
                    self.assert(
-                        (account_info.data.free + account_info.data.reserved) >= 200,
+                        (account_info.data.free + account_info.data.reserved)
+                            >= EXISTENTIAL_DEPOSIT,
                        format!(
                            "Account {} not respect existential deposit rule.",
                            account_id
@@ -198,9 +213,9 @@ mod verifier {
                    );
                }

-                // Rule 3: If the account have consumers, it shoul have at least one provider
+                // Rule 3: If the account have consumers, it should have at least one provider
                if account_info.consumers > 0 {
-                    // Rule 1: If the account is not s
+                    // Rule 1: If the account is not sufficient [...]
                    self.assert(
                        account_info.providers > 0,
                        format!("Account {} has no providers nor sufficients.", account_id),
@@ -218,12 +233,31 @@ mod verifier {
            }
        }

+        /// check list of identities (account existence, sufficient)
        async fn verify_identities(
            &mut self,
            accounts: &HashMap<AccountId32, AccountInfo>,
            identities: &HashMap<IdtyIndex, IdtyValue>,
        ) {
+            // counts occurence of owner key
+            let mut countmap = CountMap::<AccountId32, u8>::new();
+            // list owner key with multiple occurences
+            let mut duplicates = HashSet::new();
+
            for (idty_index, idty_value) in identities {
+                countmap.insert_or_increment(idty_value.owner_key.clone());
+                if let Some(count) = countmap.get_count(&idty_value.owner_key) {
+                    if count > 1 {
+                        self.error(format!(
+                            "address {} is the owner_key of {count} identities",
+                            idty_value.owner_key
+                        ));
+                        if count == 2 {
+                            duplicates.insert(idty_value.owner_key.clone());
+                        }
+                    }
+                }
+
                // Rule 1: each identity should have an account
                let maybe_account = accounts.get(&idty_value.owner_key);
                self.assert(
@@ -244,7 +278,7 @@ mod verifier {

                match idty_value.status {
                    IdtyStatus::Validated => {
-                        // Rule 3: If the identity is validated, removable_on shoud be zero
+                        // Rule 3: If the identity is validated, removable_on should be zero
                        self.assert(
                            idty_value.removable_on == 0,
                            format!(
@@ -254,7 +288,7 @@ mod verifier {
                        );
                    }
                    _ => {
-                        // Rule 4: If the identity is not validated, next_creatable_identity_on shoud be zero
+                        // Rule 4: If the identity is not validated, next_creatable_identity_on should be zero
                        self.assert(
 							idty_value.next_creatable_identity_on == 0,
 							format!("Identity {} is corrupted: next_creatable_identity_on > 0 on non-validated idty",
@@ -263,8 +297,18 @@ mod verifier {
                    }
                }
            }
+
+            for (idty_index, idty_value) in identities {
+                if duplicates.contains(&idty_value.owner_key) {
+                    self.error(format!(
+                        "duplicate key {} at position {idty_index}",
+                        idty_value.owner_key
+                    ));
+                }
+            }
        }

+        /// check the identity hashmap (length, identity existence, hash matches owner key)
        async fn verify_identity_index_of(
            &mut self,
            identities: &HashMap<IdtyIndex, IdtyValue>,
@@ -273,7 +317,11 @@ mod verifier {
            // Rule1: identity_index_of should have the same lenght as identities
            self.assert(
                identities.len() == identity_index_of.len(),
-                "identities.len() != identity_index_of.len().".to_owned(),
+                format!(
+                    "identities.len({}) != identity_index_of.len({}).",
+                    identities.len(),
+                    identity_index_of.len()
+                ),
            );

            for (blake2_128_owner_key, idty_index) in identity_index_of {
@@ -301,5 +349,29 @@ mod verifier {
                }
            }
        }
+
+        /// check coherence between identity list and identity index hashmap
+        async fn verify_identity_coherence(
+            &mut self,
+            identities: &HashMap<IdtyIndex, IdtyValue>,
+            identity_index_of: &HashMap<[u8; 16], IdtyIndex>,
+        ) {
+            // each identity should be correcly referenced in the hashmap
+            for (idty_index, idty_value) in identities {
+                // hash owner key to get key
+                let blake2_128_owner_key = &blake2_128(idty_value.owner_key.as_slice());
+
+                // get identity index from hashmap
+                if let Some(index_of) = identity_index_of.get(blake2_128_owner_key) {
+                    self.assert(idty_index == index_of,
+                        format!("identity number {idty_index} with owner key {0} is mapped to identity index {index_of}", idty_value.owner_key));
+                } else {
+                    self.error(format!(
+                        "identity with owner key {} is not present in hashmap",
+                        idty_value.owner_key
+                    ));
+                }
+            }
+        }
    }
 }