fix review comments

Author: OBorce

common/src/address/dehexify.rs

@@ -44,13 +44,14 @@ pub fn to_dehexified_json<T: serde::Serialize>(
 // TODO: add tests that create blocks, and ensure the replacement in json works properly.
 #[cfg(test)]
 mod tests {
+    use rstest::rstest;
+    use strum::{EnumCount, EnumDiscriminants, EnumIter, IntoEnumIterator};
+
     use crypto::{
         key::{KeyKind, PrivateKey},
         vrf::{VRFKeyKind, VRFPrivateKey, VRFPublicKey},
     };
-    use rstest::rstest;
-    use strum::{EnumCount, EnumDiscriminants, EnumIter, IntoEnumIterator};
-    use test_utils::random::{make_seedable_rng, Rng, Seed};
+    use test_utils::random::{gen_random_alnum_string, make_seedable_rng, Seed};
 
     use crate::{
         address::{hexified::HexifiedAddress, pubkeyhash::PublicKeyHash, Address},
@@ -61,13 +62,6 @@ mod tests {
         primitives::H256,
     };
 
-    fn random_string(length: usize, rng: &mut impl Rng) -> String {
-        rng.sample_iter(&randomness::distributions::Alphanumeric)
-            .take(length)
-            .map(char::from)
-            .collect()
-    }
-
     #[derive(PartialEq, Eq, PartialOrd, Ord, EnumCount, EnumDiscriminants)]
     #[strum_discriminants(name(HexifiableTag), derive(EnumIter))]
     enum Hexifiable {
@@ -90,51 +84,44 @@ mod tests {
         let chain_config = create_regtest();
 
         let strings = (0..100)
-            .map(|_| {
-                let size = rng.gen::<usize>() % 50;
-                random_string(size, &mut rng)
-            })
+            .map(|_| gen_random_alnum_string(&mut rng, 0, 50))
             .collect::<Vec<String>>();
 
         let keys = (0..strings.len())
-            .map(|_| {
-                //
-
-                match HexifiableTag::iter().choose(&mut rng).unwrap() {
-                    HexifiableTag::Destination => {
-                        let dest = match DestinationTag::iter().choose(&mut rng).unwrap() {
-                            DestinationTag::AnyoneCanSpend => Destination::AnyoneCanSpend,
-                            DestinationTag::PublicKey => {
-                                let (_private_key, public_key) =
-                                    PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
-                                Destination::PublicKey(public_key)
-                            }
-                            DestinationTag::PublicKeyHash => {
-                                let (_private_key, public_key) =
-                                    PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
-                                Destination::PublicKeyHash(PublicKeyHash::from(&public_key))
-                            }
-                            DestinationTag::ScriptHash => Destination::ScriptHash(
-                                crate::primitives::Id::new(H256::random_using(&mut rng)),
-                            ),
-                            DestinationTag::ClassicMultisig => {
-                                let (_private_key, public_key) =
-                                    PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
-                                Destination::ClassicMultisig(PublicKeyHash::from(&public_key))
-                            }
-                        };
-                        Hexifiable::Destination(dest)
-                    }
-                    HexifiableTag::PoolId => Hexifiable::PoolId(PoolId::random_using(&mut rng)),
-                    HexifiableTag::DelegationId => {
-                        Hexifiable::DelegationId(DelegationId::random_using(&mut rng))
-                    }
-                    HexifiableTag::TokenId => Hexifiable::TokenId(TokenId::random_using(&mut rng)),
-                    HexifiableTag::OrderId => Hexifiable::OrderId(OrderId::random_using(&mut rng)),
-                    HexifiableTag::VRFPublicKey => Hexifiable::VRFPublicKey(
-                        VRFPrivateKey::new_from_rng(&mut rng, VRFKeyKind::Schnorrkel).1,
-                    ),
+            .map(|_| match HexifiableTag::iter().choose(&mut rng).unwrap() {
+                HexifiableTag::Destination => {
+                    let dest = match DestinationTag::iter().choose(&mut rng).unwrap() {
+                        DestinationTag::AnyoneCanSpend => Destination::AnyoneCanSpend,
+                        DestinationTag::PublicKey => {
+                            let (_private_key, public_key) =
+                                PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
+                            Destination::PublicKey(public_key)
+                        }
+                        DestinationTag::PublicKeyHash => {
+                            let (_private_key, public_key) =
+                                PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
+                            Destination::PublicKeyHash(PublicKeyHash::from(&public_key))
+                        }
+                        DestinationTag::ScriptHash => Destination::ScriptHash(
+                            crate::primitives::Id::new(H256::random_using(&mut rng)),
+                        ),
+                        DestinationTag::ClassicMultisig => {
+                            let (_private_key, public_key) =
+                                PrivateKey::new_from_rng(&mut rng, KeyKind::Secp256k1Schnorr);
+                            Destination::ClassicMultisig(PublicKeyHash::from(&public_key))
+                        }
+                    };
+                    Hexifiable::Destination(dest)
+                }
+                HexifiableTag::PoolId => Hexifiable::PoolId(PoolId::random_using(&mut rng)),
+                HexifiableTag::DelegationId => {
+                    Hexifiable::DelegationId(DelegationId::random_using(&mut rng))
                 }
+                HexifiableTag::TokenId => Hexifiable::TokenId(TokenId::random_using(&mut rng)),
+                HexifiableTag::OrderId => Hexifiable::OrderId(OrderId::random_using(&mut rng)),
+                HexifiableTag::VRFPublicKey => Hexifiable::VRFPublicKey(
+                    VRFPrivateKey::new_from_rng(&mut rng, VRFKeyKind::Schnorrkel).1,
+                ),
             })
             .collect::<Vec<_>>();
 

wasm-wrappers/WASM-API.md

@@ -211,9 +211,9 @@ ScriptHash and ClassicMultisig destinations are not supported.
 Given inputs as bytes, outputs as bytes, and flags settings, this function returns
 the transaction that contains them all, as bytes.
 
-### Function: `decode_signed_transaction_to_json_str`
+### Function: `decode_signed_transaction_to_js`
 
-Decodes a signed transaction from its binary encoding into a Json string.
+Decodes a signed transaction from its binary encoding into a JavaScript object.
 
 ### Function: `encode_witness_no_signature`
 

wasm-wrappers/js-bindings-test/tests/test_transaction_and_witness_encoding.ts

@@ -21,7 +21,7 @@ import {
   estimate_transaction_size,
   make_default_account_privkey,
   make_receiving_address,
-  decode_signed_transaction_to_json_str,
+  decode_signed_transaction_to_js,
   Network,
   SignatureHashType,
 } from "../../pkg/wasm_wrappers.js";
@@ -331,10 +331,7 @@ export function test_transaction_and_witness_encoding() {
     `estimated size ${estimated_size} vs real ${expected_signed_tx.length}`,
   );
 
-  let decoded_tx = decode_signed_transaction_to_json_str(
-    signed_tx,
-    Network.Testnet,
-  );
+  let decoded_tx = decode_signed_transaction_to_js(signed_tx, Network.Testnet);
 
   const expected_decoded_tx = {
     transaction: {
@@ -421,5 +418,8 @@ export function test_transaction_and_witness_encoding() {
     ],
   };
 
-  assert_eq_vals(decoded_tx, JSON.stringify(expected_decoded_tx));
+  assert_eq_vals(
+    JSON.stringify(decoded_tx),
+    JSON.stringify(expected_decoded_tx),
+  );
 }

wasm-wrappers/src/error.rs

@@ -81,8 +81,8 @@ pub enum Error {
     #[error("Invalid signed transaction intent encoding: {0}")]
     InvalidSignedTransactionIntentEncoding(serialization::Error),
 
-    #[error("Invalid signed transaction encoding: {0}")]
-    InvalidSignedTransactionEncoding(serialization::Error),
+    #[error("JSON parsing error: {0:?}")]
+    JsonParseError(JsValue),
 
     #[error("Error creating multisig spend: {0}")]
     MultisigSpendCreationError(DestinationSigError),

wasm-wrappers/src/lib.rs

@@ -31,11 +31,9 @@
 
 use std::{num::NonZeroU8, str::FromStr};
 
-use wasm_bindgen::JsValue;
-
 use bip39::Language;
 use itertools::Itertools as _;
-use wasm_bindgen::prelude::*;
+use wasm_bindgen::{prelude::*, JsValue};
 
 use common::{
     address::{dehexify::dehexify_all_addresses, pubkeyhash::PublicKeyHash, Address},
@@ -691,21 +689,20 @@ pub fn encode_transaction(inputs: &[u8], outputs: &[u8], flags: u64) -> Result<V
     Ok(tx.encode())
 }
 
-/// Decodes a signed transaction from its binary encoding into a Json string.
+/// Decodes a signed transaction from its binary encoding into a JavaScript object.
 #[wasm_bindgen]
-pub fn decode_signed_transaction_to_json_str(
+pub fn decode_signed_transaction_to_js(
     transaction: &[u8],
     network: Network,
 ) -> Result<JsValue, Error> {
     let chain_config = Builder::new(network.into()).build();
     let tx = SignedTransaction::decode_all(&mut &transaction[..])
-        .map_err(Error::InvalidSignedTransactionEncoding)?;
+        .map_err(Error::InvalidTransactionEncoding)?;
 
     let str = JsonEncoded::new(&tx).to_string();
     let str = dehexify_all_addresses(&chain_config, &str);
 
-    let value = JsValue::from_str(&str);
-    Ok(value)
+    js_sys::JSON::parse(&str).map_err(Error::JsonParseError)
 }
 
 /// Encode an input witness of the variant that contains no signature.