chore: code rabbit review

dashpay · Jan 23, 2025 · e7213a7 · e7213a7
1 parent 2336773
commit e7213a7
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 99 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -31,7 +31,6 @@ members = [
     "packages/wallet-utils-contract",
 ]
 
-
 exclude = ["packages/wasm-sdk"] # This one is experimental and not ready for use
 
 [workspace.package]

diff --git a/packages/rs-dapi-client/Cargo.toml b/packages/rs-dapi-client/Cargo.toml
@@ -4,8 +4,8 @@ version = "1.8.0"
 edition = "2021"
 
 [features]
-# TODO we should not enable deps features in default
-default = ["mocks", "offline-testing"]
+
+default = []
 
 mocks = [
     "dep:sha2",
@@ -18,7 +18,7 @@ mocks = [
 # dump requests and responses to file
 dump = ["mocks"]
 # skip tests that require connection to the platform; enabled by default
-offline-testing = []
+online-testing = []
 
 # non-wasm dependencies
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]

diff --git a/packages/rs-dapi-client/src/transport/wasm_channel.rs b/packages/rs-dapi-client/src/transport/wasm_channel.rs
@@ -101,6 +101,10 @@ impl backon::Sleeper for WasmBackonSleeper {
 ///
 /// This is a workaround using oneshot channel to synchronize.
 /// It spawns a local task that sends the result of the future to the channel.
+///
+/// ## Panics
+///
+/// It panics if the receiver is dropped (e.g. `f` panics or is cancelled) before the sender sends the result.
 fn into_send<'a, F: Future + 'static>(f: F) -> BoxFuture<'a, F::Output>
 where
     F::Output: Send,
@@ -109,6 +113,7 @@ where
     spawn_local(async move {
         tx.send(f.await).ok();
     });
+
     rx.unwrap_or_else(|e| panic!("Failed to receive result: {:?}", e))
         .boxed()
 }
diff --git a/packages/rs-dapi-client/tests/local_platform_connectivity.rs b/packages/rs-dapi-client/tests/local_platform_connectivity.rs
@@ -1,4 +1,4 @@
-#[cfg(not(feature = "offline-testing"))]
+#[cfg(feature = "online-testing")]
 mod tests {
     use dapi_grpc::{
         platform::v0::{

diff --git a/packages/rs-sdk/Cargo.toml b/packages/rs-sdk/Cargo.toml
@@ -95,6 +95,9 @@ offline-testing = ["mocks"]
 # Requires configuration of Dash Platform connectivity.
 # See [README.md] for more details.
 #
+# Without this feature enabled, tests will use test vectors from `tests/vectors/` instead of connecting to live 
+# Dash Platform.
+#
 # If both `offline-testing` and `network-testing` are enabled, "offline-testing" will take precedence.
 network-testing = ["mocks"]
 

diff --git a/packages/rs-sdk/src/sdk.rs b/packages/rs-sdk/src/sdk.rs
@@ -838,10 +838,9 @@ impl SdkBuilder {
     ///
     /// This method is unstable and can be changed in the future.
     pub fn new_mainnet() -> Self {
-        // TODO this is just some test mainnet node, we need to implement proper lookup of nodes
-        let addresses = AddressList::from_str("https://34.223.102.43:443")
-            .expect("hardcoded mainnet addresses must be valid");
-        Self::new(addresses).with_network(Network::Dash)
+        unimplemented!(
+            "Mainnet address list not implemented yet. Use new() and provide address list."
+        )
     }
 
     /// Configure network type.

diff --git a/packages/wasm-sdk/index.html b/packages/wasm-sdk/index.html
@@ -3,83 +3,85 @@
 
 <head>
   <meta content="text/html;charset=utf-8" http-equiv="Content-Type" />
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
   <style>
     #preloader {
       display: none;
     }
+
     .container {
       padding: 10px;
     }
   </style>
 </head>
 
 <body>
-<div id="preloader">Loading...</div>
-<div class="container">
-<input type="text" id="identityInput" placeholder="Enter Identity ID" />
-<button id="fetchButton">Fetch Identity</button>
-</div>
+  <div id="preloader">Loading...</div>
+  <div class="container">
+    <input type="text" id="identityInput" placeholder="Enter Identity ID" />
+    <button id="fetchButton">Fetch Identity</button>
+  </div>
 
-<div class="container">
-<input type="text" id="identityInput2" placeholder="Enter Data Contract ID" />
-<button id="fetchButton2">Fetch Data Contract</button>
-</div>
-<div id="identityInfo"></div>
+  <div class="container">
+    <input type="text" id="identityInput2" placeholder="Enter Data Contract ID" />
+    <button id="fetchButton2">Fetch Data Contract</button>
+  </div>
+  <div id="identityInfo"></div>
 
-<script type="module">
-  import init, { start, WasmSdkBuilder, identity_fetch, data_contract_fetch } from './pkg/wasm_sdk.js';
+  <script type="module">
+    import init, { start, WasmSdkBuilder, identity_fetch, data_contract_fetch } from './pkg/wasm_sdk.js';
 
-  async function run() {
-    await init();
+    async function run() {
+      await init();
 
-    const sdk = await WasmSdkBuilder.new_mainnet().build();
-    console.log(sdk);
+      const sdk = await WasmSdkBuilder.new_mainnet().build();
+      console.log(sdk);
 
-    document.getElementById('fetchButton').addEventListener('click', async () => {
-      const identityId = document.getElementById('identityInput').value;
-      const preloader = document.getElementById('preloader');
-      const identityInfo = document.getElementById('identityInfo');
+      document.getElementById('fetchButton').addEventListener('click', async () => {
+        const identityId = document.getElementById('identityInput').value;
+        const preloader = document.getElementById('preloader');
+        const identityInfo = document.getElementById('identityInfo');
 
-      preloader.style.display = 'block';
-      identityInfo.textContent = '';
+        preloader.style.display = 'block';
+        identityInfo.textContent = '';
 
-      try {
-        const identity = await identity_fetch(sdk, identityId);
-        console.log("fetched identity");
-        console.log(identity);
-        identityInfo.textContent = JSON.stringify(identity.toJSON(), null, 2);
-      } catch (error) {
-        console.error("Error fetching identity:", error);
-        identityInfo.textContent = `Error fetching identity: ${error}`;
-      } finally {
-        preloader.style.display = 'none';
-      }
-    });
+        try {
+          const identity = await identity_fetch(sdk, identityId);
+          console.log("fetched identity");
+          console.log(identity);
+          identityInfo.textContent = JSON.stringify(identity.toJSON(), null, 2);
+        } catch (error) {
+          console.error("Error fetching identity:", error);
+          identityInfo.textContent = `Error fetching identity: ${error}`;
+        } finally {
+          preloader.style.display = 'none';
+        }
+      });
 
-    document.getElementById('fetchButton2').addEventListener('click', async () => {
-      const identityId = document.getElementById('identityInput2').value;
-      const preloader = document.getElementById('preloader');
-      const identityInfo = document.getElementById('identityInfo');
+      document.getElementById('fetchButton2').addEventListener('click', async () => {
+        const identityId = document.getElementById('identityInput2').value;
+        const preloader = document.getElementById('preloader');
+        const identityInfo = document.getElementById('identityInfo');
 
-      preloader.style.display = 'block';
-      identityInfo.textContent = '';
+        preloader.style.display = 'block';
+        identityInfo.textContent = '';
 
-      try {
-        const dataContract = await data_contract_fetch(sdk, identityId);
-        console.log("fetched data contract");
-        console.log(dataContract);
-        identityInfo.textContent = JSON.stringify(dataContract.toJSON(), null, 2);
-      } catch (error) {
-        console.error("Error fetching contract:", error);
-        identityInfo.textContent = `Error fetching contract: ${error}`;
-      } finally {
-        preloader.style.display = 'none';
-      }
-    });
-  }
+        try {
+          const dataContract = await data_contract_fetch(sdk, identityId);
+          console.log("fetched data contract");
+          console.log(dataContract);
+          identityInfo.textContent = JSON.stringify(dataContract.toJSON(), null, 2);
+        } catch (error) {
+          console.error("Error fetching contract:", error);
+          identityInfo.textContent = `Error fetching contract: ${error}`;
+        } finally {
+          preloader.style.display = 'none';
+        }
+      });
+    }
 
-  run();
-</script>
+    run();
+  </script>
 </body>
 
-</html>
+</html>
diff --git a/packages/wasm-sdk/src/dpp.rs b/packages/wasm-sdk/src/dpp.rs
@@ -1,19 +1,14 @@
 use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0};
-use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0;
-use dash_sdk::dpp::identity::{IdentityPublicKey, KeyID};
-use dash_sdk::dpp::metadata::Metadata;
 use dash_sdk::dpp::platform_value::ReplacementType;
 use dash_sdk::dpp::serialization::PlatformDeserializable;
-use dash_sdk::dpp::serialization::PlatformSerializable;
 use dash_sdk::dpp::serialization::ValueConvertible;
 
-use crate::error::{to_js_error, WasmError};
+use crate::error::to_js_error;
 use dash_sdk::dashcore_rpc::dashcore::hashes::serde::Serialize;
 use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
 use dash_sdk::dpp::data_contract::conversion::json::DataContractJsonConversionMethodsV0;
 use dash_sdk::dpp::version::PlatformVersion;
 use dash_sdk::platform::{DataContract, Identity};
-use dash_sdk::Error;
 use platform_value::string_encoding::Encoding;
 use wasm_bindgen::prelude::*;
 use wasm_bindgen::JsValue;

diff --git a/packages/wasm-sdk/src/lib.rs b/packages/wasm-sdk/src/lib.rs
@@ -10,14 +10,14 @@ pub mod verify;
 #[global_allocator]
 static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;
 
-// TODO: Remove tracing or use
-//https://github.com/old-storyai/tracing-wasm
-//https://github.com/jquesada2016/tracing_subscriber_wasm
-//https://crates.io/crates/tracing-web
-
 #[wasm_bindgen(start)]
 pub async fn start() -> Result<(), JsValue> {
+    // We use tracing-wasm together with console_error_panic_hook to get logs from the wasm module.
+    // Other alternatives are:
+    // * https://github.com/jquesada2016/tracing_subscriber_wasm
+    // * https://crates.io/crates/tracing-web
     console_error_panic_hook::set_once();
+
     tracing_wasm::set_as_global_default();
 
     Ok(())

diff --git a/packages/wasm-sdk/src/sdk.rs b/packages/wasm-sdk/src/sdk.rs
@@ -13,15 +13,15 @@ use dash_sdk::platform::transition::broadcast::BroadcastStateTransition;
 use dash_sdk::platform::transition::put_identity::PutIdentity;
 use dash_sdk::platform::{DataContract, Document, DocumentQuery, Fetch, Identifier, Identity};
 use dash_sdk::sdk::AddressList;
-use dash_sdk::{Error, Sdk, SdkBuilder};
+use dash_sdk::{Sdk, SdkBuilder};
 use platform_value::platform_value;
 use std::collections::BTreeMap;
-use std::fmt::{Debug, Display};
+use std::fmt::Debug;
 use std::ops::{Deref, DerefMut};
 use std::str::FromStr;
 use wasm_bindgen::prelude::wasm_bindgen;
 use wasm_bindgen::JsError;
-use web_sys::{console, js_sys, window};
+use web_sys::{console, js_sys};
 
 #[wasm_bindgen]
 pub struct WasmSdk(Sdk);
@@ -70,7 +70,7 @@ impl WasmSdkBuilder {
     }
 
     pub fn new_testnet() -> Self {
-        WasmSdkBuilder(SdkBuilder::new_testnet())
+        WasmSdkBuilder(SdkBuilder::new_testnet()).with_context_provider(WasmContext {})
     }
 
     pub fn build(self) -> Result<WasmSdk, JsError> {
@@ -113,6 +113,8 @@ pub async fn data_contract_fetch(
 
 #[wasm_bindgen]
 pub async fn identity_put(sdk: &WasmSdk) {
+    // This is just a mock implementation to show how to use the SDK and ensure proper linking
+    // of all required dependencies. This function is not supposed to work.
     let id = Identifier::from_bytes(&[0; 32]).expect("create identifier");
 
     let identity = Identity::V0(IdentityV0 {
@@ -148,7 +150,7 @@ pub async fn epoch_testing() {
         .build()
         .expect("build sdk");
 
-    let ei = ExtendedEpochInfo::fetch(&sdk, 0)
+    let _ei = ExtendedEpochInfo::fetch(&sdk, 0)
         .await
         .expect("fetch extended epoch info")
         .expect("extended epoch info not found");
@@ -163,7 +165,7 @@ pub async fn docs_testing(sdk: &WasmSdk) {
         .create(id, 1, platform_value!({}), None, None)
         .expect("create data contract");
 
-    let dc = DataContract::fetch(&sdk, id)
+    let dc = DataContract::fetch(sdk, id)
         .await
         .expect("fetch data contract")
         .expect("data contract not found");
@@ -201,6 +203,6 @@ impl Signer for MockSigner {
         _identity_public_key: &dash_sdk::platform::IdentityPublicKey,
         _data: &[u8],
     ) -> Result<dash_sdk::dpp::platform_value::BinaryData, dash_sdk::dpp::ProtocolError> {
-        todo!()
+        todo!("signature creation is not implemented due to lack of dash platform wallet support in wasm")
     }
 }
diff --git a/packages/wasm-sdk/src/state_transitions/documents.rs b/packages/wasm-sdk/src/state_transitions/documents.rs
@@ -1,33 +1,25 @@
 use crate::error::to_js_error;
-use dash_sdk::dashcore_rpc::jsonrpc::serde_json::Value;
-use dash_sdk::dpp::data_contract::document_type::DocumentTypeRef;
 use dash_sdk::dpp::identity::KeyID;
-use dash_sdk::dpp::platform_value::string_encoding::Encoding;
-use dash_sdk::dpp::platform_value::Bytes32;
 use dash_sdk::dpp::serialization::PlatformSerializable;
 use dash_sdk::dpp::state_transition::documents_batch_transition::document_base_transition::v0::DocumentBaseTransitionV0;
 use dash_sdk::dpp::state_transition::documents_batch_transition::document_base_transition::DocumentBaseTransition;
 use dash_sdk::dpp::state_transition::documents_batch_transition::document_create_transition::DocumentCreateTransitionV0;
-use dash_sdk::dpp::state_transition::documents_batch_transition::document_transition::action_type::DocumentTransitionActionType;
 use dash_sdk::dpp::state_transition::documents_batch_transition::document_transition::DocumentTransition;
 use dash_sdk::dpp::state_transition::documents_batch_transition::{
     DocumentCreateTransition, DocumentsBatchTransition, DocumentsBatchTransitionV0,
 };
-use dash_sdk::platform::{Document, Identifier};
-use std::collections::{BTreeMap, HashMap, HashSet};
-use std::fmt::Display;
 use wasm_bindgen::prelude::*;
-use web_sys::js_sys::{Number, Uint32Array, Uint8Array};
+use web_sys::js_sys::{Number, Uint8Array};
 
 #[wasm_bindgen]
 pub fn create_document(
-    document: JsValue,
-    identity_contract_nonce: Number,
+    _document: JsValue,
+    _identity_contract_nonce: Number,
     signature_public_key_id: Number,
 ) -> Result<Uint8Array, JsError> {
     // TODO: Extract document fields from JsValue
 
-    let base = DocumentBaseTransition::V0(DocumentBaseTransitionV0 {
+    let _base = DocumentBaseTransition::V0(DocumentBaseTransitionV0 {
         id: Default::default(),
         identity_contract_nonce: 1,
         document_type_name: "".to_string(),
@@ -55,8 +47,18 @@ fn create_batch_transition(
         .as_f64()
         .ok_or_else(|| JsError::new("public_key_id must be a number"))?;
 
-    // TODO: This is unsafe convertion
-    let signature_public_key_id = signature_public_key_id as KeyID;
+    // boundary checks
+    let signature_public_key_id = if signature_public_key_id.is_finite()
+        && signature_public_key_id >= KeyID::MIN as f64
+        && signature_public_key_id <= (KeyID::MAX as f64)
+    {
+        signature_public_key_id as KeyID
+    } else {
+        return Err(JsError::new(&format!(
+            "signature_public_key_id {} out of valid range",
+            signature_public_key_id
+        )));
+    };
 
     let document_batch_transition = DocumentsBatchTransition::V0(DocumentsBatchTransitionV0 {
         owner_id: Default::default(),

diff --git a/packages/wasm-sdk/src/verify.rs b/packages/wasm-sdk/src/verify.rs
@@ -1,8 +1,6 @@
 use dash_sdk::dpp::dashcore::Network;
-use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
 use dash_sdk::dpp::data_contract::DataContract;
 use dash_sdk::dpp::document::{Document, DocumentV0Getters};
-use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
 use dash_sdk::dpp::identity::Identity;
 use dash_sdk::dpp::platform_value::string_encoding::Encoding;
 use dash_sdk::dpp::serialization::PlatformDeserializableWithPotentialValidationFromVersionedStructure;
@@ -129,6 +127,7 @@ pub async fn verify_data_contract() -> Option<DataContractWasm> {
 
 #[wasm_bindgen]
 pub async fn verify_documents() -> Vec<DocumentWasm> {
+    // TODO: this is a dummy implementation, replace with actual verification
     let data_contract =
         DataContract::versioned_deserialize(&[13, 13, 13], false, PlatformVersion::latest())
             .expect("create data contract");
-Original file line number
+Diff line change
@@ Expand Up / @@ -31,7 +31,6 @@ members = [ @@
         "packages/wallet-utils-contract",
     ]
     exclude = ["packages/wasm-sdk"] # This one is experimental and not ready for use
     [workspace.package]
@@ Expand Down @@