fix: removed Option special treatment (#127)

* fix: removed Option special treatment * chore: small docs correction
Alzymologist · Oct 19, 2023 · ccace30 · ccace30
1 parent a847c48
commit ccace30
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 145 deletions.
diff --git a/for_tests/bifrost982 b/for_tests/bifrost982
diff --git a/src/cards.rs b/src/cards.rs
@@ -29,7 +29,6 @@ use plot_icon::generate_png_scaled_default;
 
 use crate::std::{
     borrow::ToOwned,
-    boxed::Box,
     fmt::Write,
     string::{String, ToString},
     vec::Vec,
@@ -315,7 +314,6 @@ pub enum ParsedData {
     H256(H256),
     H512(H512),
     Id(AccountId32),
-    Option(Option<Box<ParsedData>>),
     PerU16(PerU16),
     Percent(Percent),
     Permill(Permill),
@@ -538,16 +536,6 @@ impl ParsedData {
                     info_flat,
                 }]
             }
-            ParsedData::Option(option) => match option {
-                None => vec![ExtendedCard {
-                    parser_card: ParserCard::None,
-                    indent,
-                    info_flat,
-                }],
-                Some(parsed_data) => {
-                    parsed_data.card(info_flat, indent, display_balance, short_specs, spec_name)
-                }
-            },
             ParsedData::PerU16(value) => single_card!(PerU16, value, indent, info_flat),
             ParsedData::Percent(value) => single_card!(Percent, value, indent, info_flat),
             ParsedData::Permill(value) => single_card!(Permill, value, indent, info_flat),
@@ -1004,7 +992,6 @@ pub enum ParserCard {
         version: String,
     },
     Nonce(String),
-    None,
     PalletName(String),
     PerU16(PerU16),
     Percent(Percent),
@@ -1158,7 +1145,6 @@ impl ExtendedCard {
                 readable(self.indent, "Chain", &format!("{name}{version}"))
             }
             ParserCard::Nonce(a) => readable(self.indent, "Nonce", a),
-            ParserCard::None => readable(self.indent, "Option", "None"),
             ParserCard::PalletName(a) => readable(self.indent, "Pallet", a),
             ParserCard::PerU16(a) => readable(self.indent, "PerU16", &a.deconstruct().to_string()),
             ParserCard::Percent(a) => {

diff --git a/src/decoding_sci.rs b/src/decoding_sci.rs
@@ -3,7 +3,7 @@
 use bitvec::prelude::BitOrder;
 use bitvec::prelude::{BitVec, Lsb0, Msb0};
 use num_bigint::{BigInt, BigUint};
-use parity_scale_codec::{Decode, DecodeAll, OptionBool};
+use parity_scale_codec::DecodeAll;
 use primitive_types::{H160, H512};
 use scale_info::{
     form::PortableForm, interner::UntrackedSymbol, Field, Type, TypeDef, TypeDefBitSequence,
@@ -24,7 +24,7 @@ use sp_core::{
     sr25519::{Public as PublicSr25519, Signature as SignatureSr25519},
 };
 
-use crate::std::{borrow::ToOwned, boxed::Box, string::String, vec::Vec};
+use crate::std::{borrow::ToOwned, string::String, vec::Vec};
 
 #[cfg(not(feature = "std"))]
 use core::{any::TypeId, mem::size_of};
@@ -314,7 +314,6 @@ pub const CALL_INDICATOR: &str = "Call";
 /// - tuples (`TypeDef::Tuple(_)`)
 /// - compacts (`TypeDef::Compact(_)`)
 /// - calls and events (`SpecialtyTypeChecked::PalletSpecific{..}`)
-/// - options (`SpecialtyTypeChecked:Option{..}`)
 ///
 /// Of those, the parser position changes on each new iteration for:
 ///
@@ -516,66 +515,6 @@ where
             )?,
             info: propagated.info,
         }),
-        SpecialtyTypeChecked::Option(ty_symbol) => {
-            propagated.reject_compact()?;
-            let param_ty = registry.resolve_ty(ty_symbol.id, ext_memory)?;
-            match &param_ty.type_def {
-                TypeDef::Primitive(TypeDefPrimitive::Bool) => {
-                    let slice_to_decode =
-                        data.read_slice(ext_memory, *position, ENUM_INDEX_ENCODED_LEN)?;
-                    let parsed_data = match OptionBool::decode(&mut slice_to_decode.as_ref()) {
-                        Ok(OptionBool(Some(true))) => {
-                            ParsedData::Option(Some(Box::new(ParsedData::PrimitiveBool(true))))
-                        }
-                        Ok(OptionBool(Some(false))) => {
-                            ParsedData::Option(Some(Box::new(ParsedData::PrimitiveBool(false))))
-                        }
-                        Ok(OptionBool(None)) => ParsedData::Option(None),
-                        Err(_) => {
-                            return Err(ParserError::UnexpectedOptionVariant {
-                                position: *position,
-                            })
-                        }
-                    };
-                    *position += ENUM_INDEX_ENCODED_LEN;
-                    Ok(ExtendedData {
-                        data: parsed_data,
-                        info: propagated.info,
-                    })
-                }
-                _ => match data.read_byte(ext_memory, *position)? {
-                    0 => {
-                        *position += ENUM_INDEX_ENCODED_LEN;
-                        Ok(ExtendedData {
-                            data: ParsedData::Option(None),
-                            info: propagated.info,
-                        })
-                    }
-                    1 => {
-                        *position += ENUM_INDEX_ENCODED_LEN;
-                        let extended_option_data = decode_with_type::<B, E, M>(
-                            &Ty::Resolved(ResolvedTy {
-                                ty: param_ty,
-                                id: ty_symbol.id,
-                            }),
-                            data,
-                            ext_memory,
-                            position,
-                            registry,
-                            Propagated::new(),
-                        )?;
-                        propagated.add_info_slice(&extended_option_data.info);
-                        Ok(ExtendedData {
-                            data: ParsedData::Option(Some(Box::new(extended_option_data.data))),
-                            info: propagated.info,
-                        })
-                    }
-                    _ => Err(ParserError::UnexpectedOptionVariant {
-                        position: *position,
-                    }),
-                },
-            }
-        }
         SpecialtyTypeChecked::PalletSpecific {
             pallet_name,
             pallet_info,

diff --git a/src/error.rs b/src/error.rs
@@ -142,9 +142,6 @@ pub enum ParserError<E: ExternalMemory> {
     UnexpectedExtrinsicType {
         extrinsic_ty_id: u32,
     },
-    UnexpectedOptionVariant {
-        position: usize,
-    },
     V14ShortTypesIncomplete {
         old_id: u32,
     },
@@ -172,7 +169,6 @@ impl<E: ExternalMemory> ParserError<E> {
             ParserError::UnexpectedCompactInsides { id } => format!("Compact type {id} in metadata type registry has unexpected type inside compact."),
             ParserError::UnexpectedEnumVariant { position } => format!("Encountered unexpected enum variant at position {position}."),
             ParserError::UnexpectedExtrinsicType { extrinsic_ty_id } => format!("Decoding is based on assumption that extrinsic type resolves into a SCALE-encoded opaque `Vec<u8>`. Unexpected type description is found for type {extrinsic_ty_id} in metadata type registry."),
-            ParserError::UnexpectedOptionVariant { position } => format!("Encountered unexpected Option<_> variant at position {position}."),
             ParserError::V14ShortTypesIncomplete { old_id } => format!("Unable to resolve type with old id {old_id} in shortened metadata type registry."),
             ParserError::V14TypeNotResolved { id } => format!("Unable to resolve type id {id} in metadata type registry."),
             ParserError::V14TypeNotResolvedShortened { id } => format!("Unable to resolve type with updated id {id} in shortened metadata type registry."),

diff --git a/src/lib.rs b/src/lib.rs
@@ -80,8 +80,8 @@
 //!
 //! Other `Path` identifiers are checked first, and used only if the further
 //! discovered type information matches the expected one, this is the case for
-//! `Call`, `Event` and `Option`. If it does not match, the data is parsed as
-//! is, i.e. without fitting into specific item format.
+//! `Call` and `Event`. If it does not match, the data is parsed as is, i.e.
+//! without fitting into specific item format.
 //!
 //! Enums and structs contain sets of [`Field`](scale_info::Field)s. Field
 //! `name` and `type_name` may also hint at type specialty information, although

diff --git a/src/special_indicators.rs b/src/special_indicators.rs
@@ -8,18 +8,10 @@
 //! mentioned in metadata descriptors, rather than decoded as more generalized
 //! type and cast into custom type later on.
 use frame_metadata::v14::SignedExtensionMetadata;
-use scale_info::{
-    form::PortableForm, interner::UntrackedSymbol, Field, Path, Type, TypeDef, Variant,
-};
+use scale_info::{form::PortableForm, Field, Path, Type, TypeDef, Variant};
 
 use crate::std::{borrow::ToOwned, string::String, vec::Vec};
 
-#[cfg(feature = "std")]
-use std::any::TypeId;
-
-#[cfg(not(feature = "std"))]
-use core::any::TypeId;
-
 use crate::cards::Info;
 use crate::decoding_sci::pick_variant;
 use crate::traits::{AddressableBuffer, AsMetadata, ExternalMemory, ResolveType};
@@ -78,10 +70,6 @@ pub const H256: &str = "H256";
 /// [`Type`]-associated [`Path`] `ident` for [primitive_types::H512].
 pub const H512: &str = "H512";
 
-/// [`Type`]-associated [`Path`] `ident` indicating that the data to follow
-/// *may* be an option.
-pub const OPTION: &str = "Option";
-
 /// [`Type`]-associated [`Path`] `ident` for [sp_arithmetic::Perbill].
 pub const PERBILL: &str = "Perbill";
 
@@ -112,21 +100,13 @@ pub const SP_CORE_SR25519: &[&str] = &["sp_core", "sr25519"];
 /// [`Path`] `namespace` for `sp_core::ecdsa`.
 pub const SP_CORE_ECDSA: &[&str] = &["sp_core", "ecdsa"];
 
-/// [`Path`] `namespace` for [sp_runtime::generic::UncheckedExtrinsic].
+/// [`Path`] `namespace` for `sp_runtime::generic::UncheckedExtrinsic`.
 pub const UNCHECKED_EXTRINSIC_NAMESPACE: &[&str] =
     &["sp_runtime", "generic", "unchecked_extrinsic"];
 
-/// [`Type`]-associated [`Path`] `ident` for [sp_runtime::generic::UncheckedExtrinsic].
+/// [`Type`]-associated [`Path`] `ident` for `sp_runtime::generic::UncheckedExtrinsic`.
 pub const UNCHECKED_EXTRINSIC_IDENT: &str = "UncheckedExtrinsic";
 
-/// [`Variant`] name `None` that must be found for type to be processed as
-/// `Option`.
-pub const NONE: &str = "None";
-
-/// [`Variant`] name `Some` that must be found for type to be processed as
-/// `Option`.
-pub const SOME: &str = "Some";
-
 /// Extensions `identifier` from [`SignedExtensionMetadata`] for metadata spec
 /// version.
 ///
@@ -343,8 +323,6 @@ impl Hint {
 ///
 /// Does not propagate.
 ///
-/// Checks type internal structure for `Option`.
-///
 /// Type internal structure must be additionally confirmed for `Call` and
 /// `Event` before transforming into [`SpecialtyTypeChecked`], the type
 /// specialty that causes parser action.
@@ -355,7 +333,6 @@ pub enum SpecialtyTypeHinted {
     H160,
     H256,
     H512,
-    Option(UntrackedSymbol<TypeId>),
     PalletSpecific(PalletSpecificItem),
     Perbill,
     Percent,
@@ -395,35 +372,6 @@ impl SpecialtyTypeHinted {
                 a if H160.contains(&a) => Self::H160,
                 H256 => Self::H256,
                 H512 => Self::H512,
-                OPTION => {
-                    if let TypeDef::Variant(x) = &ty.type_def {
-                        if ty.type_params.len() == 1 {
-                            if let Some(ty_symbol) = ty.type_params[0].ty {
-                                let mut has_none = false;
-                                let mut has_some = false;
-                                for variant in x.variants.iter() {
-                                    if variant.index == 0 && variant.name == NONE {
-                                        has_none = true
-                                    }
-                                    if variant.index == 1 && variant.name == SOME {
-                                        has_some = true
-                                    }
-                                }
-                                if has_none && has_some && (x.variants.len() == 2) {
-                                    Self::Option(ty_symbol)
-                                } else {
-                                    Self::None
-                                }
-                            } else {
-                                Self::None
-                            }
-                        } else {
-                            Self::None
-                        }
-                    } else {
-                        Self::None
-                    }
-                }
                 PERBILL => Self::Perbill,
                 PERCENT => Self::Percent,
                 PERMILL => Self::Permill,
@@ -484,7 +432,6 @@ pub enum SpecialtyTypeChecked {
     H160,
     H256,
     H512,
-    Option(UntrackedSymbol<TypeId>),
     PalletSpecific {
         pallet_name: String,
         pallet_info: Info,
@@ -530,7 +477,6 @@ impl SpecialtyTypeChecked {
             SpecialtyTypeHinted::H160 => Self::H160,
             SpecialtyTypeHinted::H256 => Self::H256,
             SpecialtyTypeHinted::H512 => Self::H512,
-            SpecialtyTypeHinted::Option(x) => Self::Option(x),
             SpecialtyTypeHinted::PalletSpecific(item) => {
                 if let TypeDef::Variant(x) = &ty.type_def {
                     // found specific variant corresponding to pallet,