From 6d01848fa79dbca61bf53e8f49d31c8db8ad8840 Mon Sep 17 00:00:00 2001
From: SkymanOne <german@parity.io>
Date: Tue, 24 Oct 2023 11:27:11 +0100
Subject: [PATCH] refactor API

---
 crates/extrinsics/src/call.rs        |  7 +--
 crates/extrinsics/src/instantiate.rs |  7 +--
 crates/extrinsics/src/lib.rs         |  4 +-
 crates/extrinsics/src/upload.rs      |  3 +-
 crates/transcode/src/env_check.rs    | 82 +++++++++++++++++++++++++---
 crates/transcode/src/lib.rs          | 42 ++------------
 6 files changed, 84 insertions(+), 61 deletions(-)

diff --git a/crates/extrinsics/src/call.rs b/crates/extrinsics/src/call.rs
index 2ef684722..a670ec1dc 100644
--- a/crates/extrinsics/src/call.rs
+++ b/crates/extrinsics/src/call.rs
@@ -189,6 +189,7 @@ impl CallCommandBuilder<state::Message, state::ExtrinsicOptions> {
         let rpc = RpcClient::from_url(&url).await?;
         let client = OnlineClient::from_rpc_client(rpc.clone()).await?;
         let rpc = LegacyRpcMethods::new(rpc);
+        check_env_types(&client, &transcoder)?;
 
         let token_metadata = TokenMetadata::query(&rpc).await?;
 
@@ -237,7 +238,6 @@ impl CallExec {
     /// Returns the dry run simulation result of type [`ContractExecResult`], which
     /// includes information about the simulated call, or an error in case of failure.
     pub async fn call_dry_run(&self) -> Result<ContractExecResult<Balance, ()>> {
-        check_env_types(self.client(), self.transcoder())?;
         let storage_deposit_limit = self
             .opts
             .storage_deposit_limit()
@@ -269,10 +269,7 @@ impl CallExec {
     ) -> Result<DisplayEvents, ErrorVariant> {
         // use user specified values where provided, otherwise estimate
         let gas_limit = match gas_limit {
-            Some(gas_limit) => {
-                check_env_types(self.client(), self.transcoder())?;
-                gas_limit
-            }
+            Some(gas_limit) => gas_limit,
             None => self.estimate_gas().await?,
         };
         tracing::debug!("calling contract {:?}", self.contract);
diff --git a/crates/extrinsics/src/instantiate.rs b/crates/extrinsics/src/instantiate.rs
index 7d445b624..05e1d48c2 100644
--- a/crates/extrinsics/src/instantiate.rs
+++ b/crates/extrinsics/src/instantiate.rs
@@ -183,6 +183,7 @@ impl InstantiateCommandBuilder<state::ExtrinsicOptions> {
 
         let rpc_cli = RpcClient::from_url(&url).await?;
         let client = OnlineClient::from_rpc_client(rpc_cli.clone()).await?;
+        check_env_types(&client, &transcoder)?;
         let rpc = LegacyRpcMethods::new(rpc_cli);
 
         let token_metadata = TokenMetadata::query(&rpc).await?;
@@ -344,7 +345,6 @@ impl InstantiateExec {
     ) -> Result<
         ContractInstantiateResult<<DefaultConfig as Config>::AccountId, Balance, ()>,
     > {
-        check_env_types(self.client(), self.transcoder())?;
         let storage_deposit_limit = self.args.storage_deposit_limit;
         let call_request = InstantiateRequest {
             origin: account_id(&self.signer),
@@ -438,10 +438,7 @@ impl InstantiateExec {
     ) -> Result<InstantiateExecResult, ErrorVariant> {
         // use user specified values where provided, otherwise estimate
         let gas_limit = match gas_limit {
-            Some(gas_limit) => {
-                check_env_types(self.client(), self.transcoder())?;
-                gas_limit
-            }
+            Some(gas_limit) => gas_limit,
             None => self.estimate_gas().await?,
         };
         match self.args.code.clone() {
diff --git a/crates/extrinsics/src/lib.rs b/crates/extrinsics/src/lib.rs
index 857d1f264..828d5221c 100644
--- a/crates/extrinsics/src/lib.rs
+++ b/crates/extrinsics/src/lib.rs
@@ -378,8 +378,8 @@ fn check_env_types<T>(
 where
     T: Config,
 {
-    compare_node_env_with_contract(client.metadata().types(), transcoder)
-        .map_err(|e| anyhow!("Verification of types match failed: {:?}", e))
+    compare_node_env_with_contract(client.metadata().types(), transcoder.metadata())
+        .map_err(|e| e.into())
 }
 
 #[derive(serde::Serialize)]
diff --git a/crates/extrinsics/src/upload.rs b/crates/extrinsics/src/upload.rs
index 87919159f..ca7d7442b 100644
--- a/crates/extrinsics/src/upload.rs
+++ b/crates/extrinsics/src/upload.rs
@@ -117,6 +117,7 @@ impl UploadCommandBuilder<state::ExtrinsicOptions> {
         let url = self.opts.extrinsic_opts.url();
         let rpc_cli = RpcClient::from_url(&url).await?;
         let client = OnlineClient::from_rpc_client(rpc_cli.clone()).await?;
+        check_env_types(&client, &transcoder)?;
         let rpc = LegacyRpcMethods::new(rpc_cli);
 
         let token_metadata = TokenMetadata::query(&rpc).await?;
@@ -151,7 +152,6 @@ impl UploadExec {
     /// then sends the request using the provided URL. This operation does not modify
     /// the state of the blockchain.
     pub async fn upload_code_rpc(&self) -> Result<CodeUploadResult<CodeHash, Balance>> {
-        check_env_types(self.client(), self.transcoder())?;
         let storage_deposit_limit = self
             .opts
             .storage_deposit_limit()
@@ -174,7 +174,6 @@ impl UploadExec {
     /// The function handles the necessary interactions with the blockchain's runtime
     /// API to ensure the successful upload of the code.
     pub async fn upload_code(&self) -> Result<UploadResult, ErrorVariant> {
-        check_env_types(self.client(), self.transcoder())?;
         let storage_deposit_limit = self
             .opts
             .compact_storage_deposit_limit(&self.token_metadata)?;
diff --git a/crates/transcode/src/env_check.rs b/crates/transcode/src/env_check.rs
index af7b25bb5..1e94910d2 100644
--- a/crates/transcode/src/env_check.rs
+++ b/crates/transcode/src/env_check.rs
@@ -1,3 +1,4 @@
+use ink_metadata::InkProject;
 use scale_info::{
     form::PortableForm,
     Field,
@@ -10,14 +11,33 @@ use anyhow::{
     Result,
 };
 
-use crate::ContractMessageTranscoder;
-
 #[derive(Debug, Clone)]
 pub enum EnvCheckError {
     CheckFailed(String),
     RegistryError(String),
 }
 
+impl std::fmt::Display for EnvCheckError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::CheckFailed(e) => {
+                f.write_fmt(format_args!(
+                    "Type check failed with following error: {}",
+                    e
+                ))
+            }
+            Self::RegistryError(e) => {
+                f.write_fmt(format_args!(
+                    "Error occurred while parsing type registry: {}",
+                    e
+                ))
+            }
+        }
+    }
+}
+
+impl std::error::Error for EnvCheckError {}
+
 impl From<anyhow::Error> for EnvCheckError {
     fn from(value: anyhow::Error) -> Self {
         Self::RegistryError(value.to_string())
@@ -38,7 +58,7 @@ fn get_node_env_fields(registry: &PortableRegistry) -> Result<Vec<Field<Portable
     }
 }
 
-pub fn resolve_type_definition(
+pub(crate) fn resolve_type_definition(
     registry: &PortableRegistry,
     id: u32,
 ) -> Result<TypeDef<PortableForm>> {
@@ -47,7 +67,6 @@ pub fn resolve_type_definition(
         .context("Type is not present in registry")?;
     if tt.type_params.is_empty() {
         if let TypeDef::Composite(comp) = &tt.type_def {
-            println!("Resolve type definition: {:#?}", tt);
             let tt_id = comp
                 .fields
                 .get(0)
@@ -69,21 +88,66 @@ pub fn resolve_type_definition(
     }
 }
 
+/// Compares the environment type of the targeted chain against the current contract.
+///
+/// It is achieved by iterating over the type specifications of `Environment` trait
+/// in the node's metadata anf comparing finding the corresponding type
+/// in the contract's `Environment` trait.
 pub fn compare_node_env_with_contract(
-    registry: &PortableRegistry,
-    transcoder: &ContractMessageTranscoder,
+    node_registry: &PortableRegistry,
+    contract_metadata: &InkProject,
 ) -> Result<(), EnvCheckError> {
-    let env_fields = get_node_env_fields(registry)?;
+    let env_fields = get_node_env_fields(node_registry)?;
     for field in env_fields {
         let field_name = field.name.context("Field does not have a name")?;
         if &field_name == "hasher" {
             continue
         }
-        let field_def = resolve_type_definition(registry, field.ty.id)?;
-        let checked = transcoder.compare_type(&field_name, field_def, registry)?;
+        let field_def = resolve_type_definition(node_registry, field.ty.id)?;
+        let checked =
+            compare_type(&field_name, field_def, contract_metadata, node_registry)?;
         if !checked {
             return Err(EnvCheckError::CheckFailed(field_name))
         }
     }
     Ok(())
 }
+
+/// Compares the contract's environment type with a provided type definition.
+fn compare_type(
+    type_name: &str,
+    type_def: TypeDef<PortableForm>,
+    contract_metadata: &InkProject,
+    node_registry: &PortableRegistry,
+) -> Result<bool> {
+    let contract_registry = contract_metadata.registry();
+    let tt_id = match type_name {
+        "account_id" => contract_metadata.spec().environment().account_id().ty().id,
+        "balance" => contract_metadata.spec().environment().balance().ty().id,
+        "hash" => contract_metadata.spec().environment().hash().ty().id,
+        "timestamp" => contract_metadata.spec().environment().timestamp().ty().id,
+        "block_number" => {
+            contract_metadata
+                .spec()
+                .environment()
+                .block_number()
+                .ty()
+                .id
+        }
+        _ => anyhow::bail!("Trying to resolve unknown environment type"),
+    };
+    let tt_def = resolve_type_definition(contract_registry, tt_id)?;
+    if let TypeDef::Array(node_arr) = &type_def {
+        let node_arr_type =
+            resolve_type_definition(node_registry, node_arr.type_param.id)?;
+        if let TypeDef::Array(contract_arr) = &tt_def {
+            if node_arr.len != contract_arr.len {
+                anyhow::bail!("Mismatch in array lengths");
+            }
+            let contract_arr_type =
+                resolve_type_definition(contract_registry, contract_arr.type_param.id)?;
+            return Ok(contract_arr_type == node_arr_type)
+        }
+    }
+    Ok(type_def == tt_def)
+}
diff --git a/crates/transcode/src/lib.rs b/crates/transcode/src/lib.rs
index cdff820f0..9d2b838a0 100644
--- a/crates/transcode/src/lib.rs
+++ b/crates/transcode/src/lib.rs
@@ -107,8 +107,6 @@ mod scon;
 mod transcoder;
 mod util;
 
-use crate::env_check::resolve_type_definition;
-
 pub use self::{
     account_id::AccountId32,
     env_check::compare_node_env_with_contract,
@@ -145,8 +143,6 @@ use scale_info::{
         PortableForm,
     },
     Field,
-    PortableRegistry,
-    TypeDef,
 };
 use std::{
     cmp::Ordering,
@@ -272,6 +268,10 @@ impl ContractMessageTranscoder {
             .decode(self.metadata.registry(), type_id, input)
     }
 
+    pub fn metadata(&self) -> &InkProject {
+        &self.metadata
+    }
+
     fn constructors(&self) -> impl Iterator<Item = &ConstructorSpec<PortableForm>> {
         self.metadata.spec().constructors().iter()
     }
@@ -279,40 +279,6 @@ impl ContractMessageTranscoder {
     fn messages(&self) -> impl Iterator<Item = &MessageSpec<PortableForm>> {
         self.metadata.spec().messages().iter()
     }
-    /// Compares the contract's environment type with a provided type definition.
-    pub fn compare_type(
-        &self,
-        type_name: &str,
-        type_def: TypeDef<PortableForm>,
-        node_registry: &PortableRegistry,
-    ) -> Result<bool> {
-        let contract_registry = self.metadata.registry();
-        let tt_id = match type_name {
-            "account_id" => self.metadata.spec().environment().account_id().ty().id,
-            "balance" => self.metadata.spec().environment().balance().ty().id,
-            "hash" => self.metadata.spec().environment().hash().ty().id,
-            "timestamp" => self.metadata.spec().environment().timestamp().ty().id,
-            "block_number" => self.metadata.spec().environment().block_number().ty().id,
-            _ => anyhow::bail!("Trying to resolve unknown environment type"),
-        };
-        let tt_def = resolve_type_definition(contract_registry, tt_id)?;
-        if let TypeDef::Array(node_arr) = &type_def {
-            let node_arr_type =
-                resolve_type_definition(node_registry, node_arr.type_param.id)?;
-            if let TypeDef::Array(contract_arr) = &tt_def {
-                if node_arr.len != contract_arr.len {
-                    anyhow::bail!("Mismatch in array lengths");
-                }
-                let contract_arr_type = resolve_type_definition(
-                    contract_registry,
-                    contract_arr.type_param.id,
-                )?;
-                return Ok(contract_arr_type == node_arr_type)
-            }
-        }
-        println!("Node {:?}\nContract: {:?}", type_def, tt_def);
-        Ok(type_def == tt_def)
-    }
 
     fn find_message_spec(&self, name: &str) -> Option<&MessageSpec<PortableForm>> {
         self.messages().find(|msg| msg.label() == &name.to_string())