feat: backend: generate_message warns if metadata seems useless (#921)

* feat: backend: generate_message warns if metadata seems useless

* fix: backend: also check for duplicated extrinsics in metadata; tests

rust/definitions/src/error.rs

@@ -325,9 +325,9 @@ impl ErrorSource for Active {
                             SpecsError::NoBase58Prefix => String::from("No base58 prefix."),
                             SpecsError::Base58PrefixMismatch{specs, meta} => format!("Base58 prefix from fetched properties {} does not match base58 prefix in fetched metadata {}.", specs, meta),
                             SpecsError::Base58PrefixFormatNotSupported{value} => format!("Base58 prefix {} does not fit into u16.", value),
-                            SpecsError::NoDecimals => String::from("No decimals."),
+                            SpecsError::UnitNoDecimals(x) => format!("Network has units declared: {}, but no decimals.", x),
                             SpecsError::DecimalsFormatNotSupported{value} => format!("Decimals value {} does not fit into u8.", value),
-                            SpecsError::NoUnit => String::from("No units."),
+                            SpecsError::DecimalsNoUnit(x) => format!("Network has decimals declared: {}, but no units.", x),
                             SpecsError::UnitFormatNotSupported{value} => format!("Units {} are not String.", value),
                             SpecsError::DecimalsArrayUnitsNot => String::from("Unexpected result for multi-token network. Decimals are fetched as an array with more than one element, whereas units are not."),
                             SpecsError::DecimalsUnitsArrayLength{decimals, unit} => format!("Unexpected result for multi-token network. Length of decimals array {} does not match the length of units array {}.", decimals, unit),
@@ -635,9 +635,9 @@ pub enum SpecsError {
     NoBase58Prefix,
     Base58PrefixMismatch{specs: u16, meta: u16},
     Base58PrefixFormatNotSupported{value: String},
-    NoDecimals,
+    UnitNoDecimals(String),
     DecimalsFormatNotSupported{value: String},
-    NoUnit,
+    DecimalsNoUnit(u8),
     UnitFormatNotSupported{value: String},
     DecimalsArrayUnitsNot,
     DecimalsUnitsArrayLength{decimals: String, unit: String},

rust/definitions/src/history.rs

@@ -369,6 +369,7 @@ pub fn all_events_preview() -> Vec<Event> {
         name: String::from("westend"),
         version: 9000,
         optional_base58prefix: Some(42),
+        warn_incomplete_extensions: false,
         meta: Vec::new(),
     };
     let public = [142, 175, 4, 21, 22, 135, 115, 99, 38, 201, 254, 161, 126, 37, 252, 82, 135, 97, 54, 147, 201, 18, 144, 156, 178, 38, 170, 71, 148, 242, 106, 72];

rust/definitions/src/metadata.rs

@@ -1,8 +1,9 @@
 use parity_scale_codec::{Decode, Encode};
 use parity_scale_codec_derive;
-use frame_metadata::{RuntimeMetadata, decode_different::DecodeDifferent};
+use frame_metadata::{RuntimeMetadata, decode_different::DecodeDifferent, v14::RuntimeMetadataV14};
 use sled::IVec;
 use sp_version::RuntimeVersion;
+use std::collections::HashMap;
 
 use crate::crypto::Encryption;
 use crate::error::{Active, DatabaseActive, EntryDecodingActive, ErrorActive, ErrorSigner, ErrorSource, IncomingMetadataSourceActive, IncomingMetadataSourceActiveStr, MetadataError, MetadataSource, NotHexActive, Signer};
@@ -16,6 +17,7 @@ pub struct MetaInfo {
     pub name: String,
     pub version: u32,
     pub optional_base58prefix: Option<u16>,
+    pub warn_incomplete_extensions: bool,
 }
 
 /// Struct to store the metadata values (network name, network
@@ -25,6 +27,7 @@ pub struct MetaValues {
     pub name: String,
     pub version: u32,
     pub optional_base58prefix: Option<u16>,
+    pub warn_incomplete_extensions: bool,
     pub meta: Vec<u8>,
 }
 
@@ -47,6 +50,7 @@ impl MetaValues {
             name: meta_info.name.to_string(),
             version: meta_info.version,
             optional_base58prefix: meta_info.optional_base58prefix,
+            warn_incomplete_extensions: meta_info.warn_incomplete_extensions,
             meta: meta_vec.to_vec(),
         })
     }
@@ -59,6 +63,7 @@ impl MetaValues {
             name: meta_info.name.to_string(),
             version: meta_info.version,
             optional_base58prefix: meta_info.optional_base58prefix,
+            warn_incomplete_extensions: meta_info.warn_incomplete_extensions,
             meta: [vec![109, 101, 116, 97], runtime_metadata.encode()].concat(),
         })
     }
@@ -85,6 +90,7 @@ impl MetaValues {
 pub fn info_from_metadata (runtime_metadata: &RuntimeMetadata) -> Result<MetaInfo, MetadataError> {
     let mut runtime_version_encoded: Option<Vec<u8>> = None;
     let mut base58_prefix_encoded: Option<Vec<u8>> = None;
+    let mut warn_incomplete_extensions = false;
     let mut system_block = false;
     match runtime_metadata {
         RuntimeMetadata::V12(metadata_v12) => {
@@ -138,6 +144,7 @@ pub fn info_from_metadata (runtime_metadata: &RuntimeMetadata) -> Result<MetaInf
                 break;
                 }
             }
+            warn_incomplete_extensions = need_v14_warning(metadata_v14);
         },
         _ => return Err(MetadataError::VersionIncompatible),
     }
@@ -164,6 +171,7 @@ pub fn info_from_metadata (runtime_metadata: &RuntimeMetadata) -> Result<MetaInf
         name: runtime_version.spec_name.to_string(),
         version: runtime_version.spec_version,
         optional_base58prefix,
+        warn_incomplete_extensions,
     })
 }
 
@@ -176,7 +184,24 @@ pub fn runtime_metadata_from_vec(meta_vec: &Vec<u8>) -> Result<RuntimeMetadata,
     }
 }
 
-/// Struct for processing 
+/// Function to check if the v14 metadata has all signed extensions required for transaction decoding.
+/// True if extensions are incomplete.
+/// Currently, the decoding of the transaction demands that metadata version, network genesis hash,
+/// and era are among signed extensions. Otherwise, a ParserMetadataError would occur on decoding.
+/// However, we can not simply forbid the loading of the metadata without required set of
+/// signed extensions into Signer.
+/// This function should be used for warnings only on generate_message side and during metadata
+/// loading into Signer.
+fn need_v14_warning (metadata_v14: &RuntimeMetadataV14) -> bool {
+    let mut signed_extensions = HashMap::new();
+    for x in metadata_v14.extrinsic.signed_extensions.iter() {
+        let count = signed_extensions.entry(x.identifier.to_string()).or_insert(0);
+        *count +=1;
+    }
+    !(signed_extensions.get("CheckSpecVersion") == Some(&1) && signed_extensions.get("CheckGenesis") == Some(&1) && signed_extensions.get("CheckMortality") == Some(&1)) // no warning needed if each one encountered, and only once
+}
+
+/// Struct to keep metadata and its info for transaction decoding
 pub struct MetaSetElement {
     pub name: String,
     pub version: u32,
@@ -356,5 +381,25 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn westend9150() {
+        let filename = String::from("for_tests/westend9150");
+        let meta = read_to_string(&filename).unwrap();
+        let meta_values = MetaValues::from_str_metadata(&meta.trim(), IncomingMetadataSourceActiveStr::Default{filename}).unwrap();
+        assert!(meta_values.name == String::from("westend"), "Unexpected network name: {}", meta_values.name);
+        assert!(meta_values.version == 9150, "Unexpected network name: {}", meta_values.version);
+        assert!(!meta_values.warn_incomplete_extensions, "Expected complete extensions in westend9150.")
+    }
+
+    #[test]
+    fn shell200() {
+        let filename = String::from("for_tests/shell200");
+        let meta = read_to_string(&filename).unwrap();
+        let meta_values = MetaValues::from_str_metadata(&meta.trim(), IncomingMetadataSourceActiveStr::Default{filename}).unwrap();
+        assert!(meta_values.name == String::from("shell"), "Unexpected network name: {}", meta_values.name);
+        assert!(meta_values.version == 200, "Unexpected network name: {}", meta_values.version);
+        assert!(meta_values.warn_incomplete_extensions, "Expected incomplete extensions warning in shell200.")
+    }
 }
 

rust/generate_message/address_book

@@ -34,7 +34,7 @@ wss://kusama.kylin-node.co.uk				rejects call ("Error in the WebSocket handshake
 wss://quartz.unique.network				ok
 wss://kusama.rpc.robonomics.network/			ok
 wss://rpc.shiden.astar.network				ok
-wss://ws.parachain-collator-1.c1.sora2.soramitsu.co.jp	no decimals nor units
+wss://ws.parachain-collator-1.c1.sora2.soramitsu.co.jp	ok, no token
 wss://gamma.subgame.org/				ok
 wss://rpc.kusama.standard.tech				ok
 
@@ -48,7 +48,7 @@ wss://rpc.westend.standard.tech				ok
 wss://westend.kilt.io:9977				ok
 
 wss://rococo-rpc.polkadot.io				ok
-wss://rococo-statemint-rpc.polkadot.io			no decimals nor units
+wss://rococo-statemint-rpc.polkadot.io			ok, no token
 wss://rococo-canvas-rpc.polkadot.io			ok
 
 wss://ws.azero.dev					ok
@@ -114,7 +114,7 @@ wss://rpc.opportunity.standard.tech			ok
 wss://parachain-rpc.origin-trail.network		no units, but there are decimals
 wss://pangolin-rpc.darwinia.network			ok, token set
 wss://pangoro-rpc.darwinia.network			ok, token set
-wss://blockchain.polkadex.trade				no decimals nor units
+wss://blockchain.polkadex.trade				ok, no token
 wss://testnet-rpc.polymesh.live				ok
 wss://testnet.pontem.network/ws				ok
 wss://testnet.psm.link					rejects call(502), seems to be down on polkadot.js as well
@@ -133,7 +133,7 @@ wss://testnet.uniarts.network				base58 conflict
 wss://testnet2.unique.network				ok
 wss://vodka.rpc.neatcoin.org/ws				ok
 wss://testnet.web3games.org				ok
-wss://test1.zcloak.network				no decimals nor units
+wss://test1.zcloak.network				ok, no token
 wss://bsr.zeitgeist.pm					ok
 wss://alphaville.zero.io				ok
 

rust/generate_message/src/interpret_specs.rs

@@ -41,14 +41,14 @@ pub fn interpret_properties (x: &Map<String, JsonValue>, optional_prefix_from_me
             }
         },
     };
-    let decimals: u8 = match x.get("tokenDecimals") {
+    let decimals: Option<u8> = match x.get("tokenDecimals") {
         Some(a) => {
             match a {
                 JsonValue::Number(b) => {
                     match b.as_u64() {
                         Some(c) => {
                             match c.try_into() {
-                                Ok(d) => d,
+                                Ok(d) => Some(d),
                                 Err(_) => return Err(SpecsError::DecimalsFormatNotSupported{value: a.to_string()}),
                             }
                         },
@@ -61,7 +61,7 @@ pub fn interpret_properties (x: &Map<String, JsonValue>, optional_prefix_from_me
                             match c.as_u64() {
                                 Some(d) => {
                                     match d.try_into() {
-                                        Ok(f) => f,
+                                        Ok(f) => Some(f),
                                         Err(_) => return Err(SpecsError::DecimalsFormatNotSupported{value: a.to_string()}),
                                     }
                                 },
@@ -73,30 +73,31 @@ pub fn interpret_properties (x: &Map<String, JsonValue>, optional_prefix_from_me
                     else {
                         token_array = Some((a.to_string(), b.len()));
                         if let Some(ref token_override) = optional_token_override {
-                            token_override.decimals
+                            Some(token_override.decimals)
                         }
-                        else {0}
+                        else {Some(0)}
                     }
                 },
+                JsonValue::Null => None,
                 _ => return Err(SpecsError::DecimalsFormatNotSupported{value: a.to_string()}),
             }
         },
-        None => return Err(SpecsError::NoDecimals),
+        None => None,
     };
     let unit = match x.get("tokenSymbol") {
         Some(a) => {
             match a {
                 JsonValue::String(b) => {
                     if let Some(_) = token_array {return Err(SpecsError::DecimalsArrayUnitsNot)}
                     if let Some(_) = optional_token_override {return Err(SpecsError::OverrideIgnored)}
-                    b.to_string()
+                    Some(b.to_string())
                 },
                 JsonValue::Array(b) => {
                     if b.len() == 1 {
                         if let JsonValue::String(c) = &b[0] {
                             if let Some(_) = token_array {return Err(SpecsError::DecimalsArrayUnitsNot)}
                             if let Some(_) = optional_token_override {return Err(SpecsError::OverrideIgnored)}
-                            c.to_string()
+                            Some(c.to_string())
                         }
                         else {return Err(SpecsError::DecimalsFormatNotSupported{value: a.to_string()})}
                     }
@@ -107,22 +108,36 @@ pub fn interpret_properties (x: &Map<String, JsonValue>, optional_prefix_from_me
                                 else {
                                     if let Some(token_override) = optional_token_override {
                                         println!("Network supports several tokens. An array of tokenDecimals {} and an array of tokenSymbol {} were fetched. Through override, the decimals value will be set to {} and unit value will be set to {}. To improve this behavior, please file a ticket.", decimals, a.to_string(), token_override.decimals, token_override.unit);
-                                        token_override.unit
+                                        Some(token_override.unit)
                                     }
                                     else {
                                         println!("Network supports several tokens. An array of tokenDecimals {} and an array of tokenSymbol {} were fetched. By default, decimals value will be set to 0, and unit value will be set to UNIT. To override, use -token <value_decimals> <value_unit>. To improve this behavior, please file a ticket.", decimals, a.to_string());
-                                        String::from("UNIT")
+                                        Some(String::from("UNIT"))
                                     }
                                 }
                             },
                             None => {return Err(SpecsError::UnitsArrayDecimalsNot)},
                         }
                     }
                 },
+                JsonValue::Null => None,
                 _ => return Err(SpecsError::UnitFormatNotSupported{value: a.to_string()}),
             }
         },
-        None => return Err(SpecsError::NoUnit),
+        None => None,
+    };
+    let (decimals, unit) = match decimals {
+        Some(a) => match unit {
+            Some(b) => (a,b),
+            None => return Err(SpecsError::DecimalsNoUnit(a)),
+        },
+        None => match unit {
+            Some(b) => return Err(SpecsError::UnitNoDecimals(b)),
+            None => {
+                println!("Network has no token. By default, decimals value will be set to 0, and unit value will be set to UNIT. To improve this behavior, please file a ticket.");
+                (0, String::from("UNIT"))
+            },
+        },
     };
     Ok(NetworkProperties {
         base58prefix,

rust/generate_message/src/load.rs

@@ -93,6 +93,7 @@ fn meta_f_a_element (set_element: &AddressSpecs) -> Result<(), ErrorActive> {
     for x in metadata.scan_prefix(meta_key_prefix.prefix()) {
         if let Ok(a) = x {
             let meta_values = MetaValues::from_entry_checked::<Active>(a)?;
+            if meta_values.warn_incomplete_extensions {warn(&meta_values.name, meta_values.version);}
             if let Some(prefix_from_meta) = meta_values.optional_base58prefix {
                 if prefix_from_meta != set_element.base58prefix {
                     return Err(<Active>::faulty_metadata(MetadataError::Base58PrefixSpecsMismatch{specs: set_element.base58prefix, meta: prefix_from_meta}, MetadataSource::Database{name: meta_values.name.to_string(), version: meta_values.version}))
@@ -149,6 +150,7 @@ fn meta_d_n (name: &str) -> Result<(), ErrorActive> {
 fn meta_d_u (address: &str) -> Result<(), ErrorActive> {
     if let Ok(a) = filter_address_book_by_url(address) {if a.len() != 0 {println!("Database contains entries for {} at {}. With `-d` setting key the fetched metadata integrity with existing database entries is not checked.", a[0].name, address)}}
     let shortcut = meta_shortcut(address)?;
+    if shortcut.meta_values.warn_incomplete_extensions {warn(&shortcut.meta_values.name, shortcut.meta_values.version);}
     load_meta_print(&shortcut)
 }
 
@@ -284,9 +286,15 @@ fn shortcut_set_element (set_element: &AddressSpecs) -> Result<MetaShortCut, Err
             return Err(<Active>::faulty_metadata(MetadataError::Base58PrefixSpecsMismatch{specs: set_element.base58prefix, meta: prefix_from_meta}, MetadataSource::Incoming(IncomingMetadataSourceActive::Str(IncomingMetadataSourceActiveStr::Fetch{url: set_element.address.to_string()}))))
         }
     }
+    if shortcut.meta_values.warn_incomplete_extensions {warn(&shortcut.meta_values.name, shortcut.meta_values.version);}
     Ok(shortcut)
 }
 
+/// Print warning if the metadata (v14) has incomplete set of signed extensions
+fn warn(name: &str, version: u32) {
+    println!("Warning. Metadata {}{} has incomplete set of signed extensions, and could cause Signer to fail in parsing signable transactions using this metadata.", name, version);
+}
+
 /// Function to process .wasm files into signable entities and add metadata into the database
 pub fn unwasm (filename: &str, update_db: bool) -> Result<(), ErrorActive> {
     let testbed = match WasmTestBed::new(&Source::File(PathBuf::from(&filename))) {

rust/transaction_parsing/src/cards.rs

@@ -38,6 +38,7 @@ pub (crate) enum Warning <'a> {
     VerifierGeneralSuper{verifier_key: &'a VerifierKey, hold: &'a Hold},
     TypesAlreadyThere,
     NetworkSpecsAlreadyThere(&'a str), // network title
+    MetadataExtensionsIncomplete,
 }
 
 impl <'a> Warning <'a> {
@@ -55,6 +56,7 @@ impl <'a> Warning <'a> {
             Warning::VerifierGeneralSuper{verifier_key, hold} => format!("Received message is verified. Currently no verifier is set for network with genesis hash {} and no general verifier is set. Proceeding will update the network verifier to general. All previously acquired network information that was received unverified will be purged. {}", hex::encode(verifier_key.genesis_hash()), hold.show()),
             Warning::TypesAlreadyThere => String::from("Received types information is identical to the one that was in the database."),
             Warning::NetworkSpecsAlreadyThere (x) => format!("Received network specs information for {} is same as the one already in the database.", x),
+            Warning::MetadataExtensionsIncomplete => String::from("Received metadata has incomplete set of signed extensions. As a result, Signer may be unable to parse signable transactions using this metadata."),
         }
     }
 }