Tests for encode_partially_signed_transaction; code cleanup

Author: ImplOfAnImpl

common/src/chain/partially_signed_transaction/additional_info.rs

@@ -26,11 +26,18 @@ use crate::{
     primitives::Amount,
 };
 
+// Note: PoolAdditionalInfo and OrderAdditionalInfo below are identical to the corresponding
+// structs in `input_commitments/info_providers.rs` (except that those don't derive Encode/Decode)
+// and basically serve the same purpose.
+// We keep them separate because:
+// 1) Technically we may want to have even more info inside partially signed transaction in
+// the future, which may not be needed by the input commitments.
+// 2) We want to be able to refactor input commitments structs without worrying about breaking
+// PartiallySignedTransaction's backward compatibility.
+
 /// Pool additional info, which must be present for each ProduceBlockFromStake UTXO consumed by
 /// the transaction. Transaction's signature commits to this info since SighashInputCommitments::V1.
-// TODO: rename to DecommissionedPoolAdditionalInfo? (or even DecommissionedPoolCommitmentInfo,
-// to suggest that it's related to commitments and isn't some arbitrary info about the pool).
-#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode)]
+#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode, serde::Serialize)]
 pub struct PoolAdditionalInfo {
     pub staker_balance: Amount,
 }
@@ -42,29 +49,23 @@ pub struct PoolAdditionalInfo {
 /// while FillOrder commitments only include the initial ones. So this info representation
 /// is not ideal, as it forces the caller to provide additional info that will not actually
 /// be used.
-// TODO: perhaps it's better to split the struct and the corresponding map into two -
-// for the initial and current balances. Also, perhaps the naming should suggest that it
-// contains commitments for signing and not just arbitrary info.
-#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode)]
+#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode, serde::Serialize)]
 pub struct OrderAdditionalInfo {
     pub initially_asked: OutputValue,
     pub initially_given: OutputValue,
     pub ask_balance: Amount,
     pub give_balance: Amount,
 }
 
-// FIXME: Rename to PtxAdditionalInfo ?
-#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode)]
+#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode, serde::Serialize)]
 pub struct TxAdditionalInfo {
-    // token_info: BTreeMap<TokenId, TokenAdditionalInfo>,
     pool_info: BTreeMap<PoolId, PoolAdditionalInfo>,
     order_info: BTreeMap<OrderId, OrderAdditionalInfo>,
 }
 
 impl TxAdditionalInfo {
     pub fn new() -> Self {
         Self {
-            // token_info: BTreeMap::new(),
             pool_info: BTreeMap::new(),
             order_info: BTreeMap::new(),
         }
@@ -89,11 +90,9 @@ impl TxAdditionalInfo {
     }
 
     pub fn join(mut self, other: Self) -> Self {
-        // self.token_info.extend(other.token_info);
         self.pool_info.extend(other.pool_info);
         self.order_info.extend(other.order_info);
         Self {
-            // token_info: self.token_info,
             pool_info: self.pool_info,
             order_info: self.order_info,
         }

common/src/chain/partially_signed_transaction/mod.rs

@@ -73,7 +73,7 @@ pub enum PartiallySignedTransactionConsistencyCheck {
 ///    via the wasm call `encode_partially_signed_transaction` and the latter pass it to
 ///    wallet-rpc-daemon. The transaction is treated as a black box, so a breaking change is
 ///    technically possible, though it'll require synchronization between multiple teams.
-#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode)]
+#[derive(Debug, Eq, PartialEq, Clone, Encode, Decode, serde::Serialize)]
 pub struct PartiallySignedTransaction {
     tx: Transaction,
     witnesses: Vec<Option<InputWitness>>,
@@ -163,8 +163,9 @@ impl PartiallySignedTransaction {
         Ok(())
     }
 
-    // FIXME tests
     fn ensure_additional_info_completeness(&self) -> Result<(), PartiallySignedTransactionError> {
+        // TODO: try to re-use the input commitments machinery here instead of doing custom checks.
+
         let ensure_order_info_present =
             |order_id: &OrderId| -> Result<_, PartiallySignedTransactionError> {
                 ensure!(
@@ -235,7 +236,6 @@ impl PartiallySignedTransaction {
                     }
                 }
                 TxInput::OrderAccountCommand(command) => {
-                    // FIXME somehow re-use input commitment machinery?
                     match command {
                         OrderAccountCommand::FillOrder(id, _)
                         | OrderAccountCommand::ConcludeOrder(id) => ensure_order_info_present(id)?,

wallet/wallet-controller/src/helpers/mod.rs

@@ -315,6 +315,7 @@ pub async fn fetch_input_infos<T: NodeInterface, B: storage::Backend>(
 
     Ok((input_utxos, ptx_additional_info, destinations))
 }
+
 async fn into_utxo_and_destination<T: NodeInterface, B: storage::Backend>(
     rpc_client: &T,
     wallet: &RuntimeWallet<B>,
@@ -402,10 +403,8 @@ fn collect_referenced_token_ids_from_ptx(
     ptx: &PartiallySignedTransaction,
     dest: &mut BTreeSet<TokenId>,
 ) {
-    for input_utxo in ptx.input_utxos() {
-        if let Some(tx_output) = input_utxo {
-            collect_referenced_token_ids_from_tx_output(tx_output, dest);
-        }
+    for input_utxo in ptx.input_utxos().iter().flatten() {
+        collect_referenced_token_ids_from_tx_output(input_utxo, dest);
     }
 
     for tx_output in ptx.tx().outputs() {

wasm-wrappers/WASM-API.md

@@ -271,6 +271,43 @@ and a network type (mainnet, testnet, etc), this function returns a witness to b
 
 Given an unsigned transaction and signatures, this function returns a SignedTransaction object as bytes.
 
+### Function: `encode_partially_signed_transaction`
+
+Return a PartiallySignedTransaction object as bytes.
+
+`transaction` is an encoded `Transaction` (which can be produced via `encode_transaction`).
+
+`signatures`, `input_utxos`, `input_destinations` and `htlc_secrets` are encoded lists of
+optional objects of the corresponding type. To produce such a list, iterate over your
+original list of optional objects and then:
+1) emit byte 0 if the current object is null;
+2) otherwise emit byte 1 followed by the object in its encoded form.
+
+Each individual object in each of the lists corresponds to the transaction input with the same
+index and its meaning is as follows:
+  1) `signatures` - the signature for the input;
+  2) `input_utxos`- the utxo for the input (if it's utxo-based);
+  3) `input_destinations` - the destination (address) corresponding to the input; this determines
+     the key(s) with which the input has to be signed. Note that for utxo-based inputs the
+     corresponding destination can usually be extracted from the utxo itself (the exception
+     being the `ProduceBlockFromStake` utxo, which doesn't contain the pool's decommission key).
+     However, PartiallySignedTransaction requires that *all* input destinations are provided
+     explicitly anyway.
+  4) `htlc_secrets` - if the input is an HTLC one and if the transaction is spending the HTLC,
+     this should be the HTLC secret. Otherwise it should be null.
+
+  The number of items in each list must be equal to the number of transaction inputs.
+
+`additional_info` has the same meaning as in `encode_witness`.
+
+### Function: `decode_partially_signed_transaction_to_js`
+
+Decodes a partially signed transaction from its binary encoding into a JavaScript object.
+
+### Function: `encode_destination`
+
+Convert the specified string address into a Destination object, encoded as bytes.
+
 ### Function: `get_transaction_id`
 
 Given a `Transaction` encoded in bytes (not a signed transaction, but a signed transaction is tolerated by ignoring the extra bytes, by choice)
@@ -338,6 +375,7 @@ Note:
    instead of `encode_witness`).
    Note that in orders v0 FillOrder inputs can technically have a signature, it's just not checked.
    But in orders V1 we actually require that those inputs don't have signatures.
+   Also, in orders V1 the provided destination is always ignored.
 
 ### Function: `encode_input_for_freeze_order`
 

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

@@ -26,6 +26,7 @@ import { test_misc } from "./test_misc.js";
 import { test_orders } from "./test_orders.js";
 import { test_signed_transaction_intent } from "./test_signed_transaction_intent.js";
 import { test_transaction_and_witness_encoding } from "./test_transaction_and_witness_encoding.js";
+import { test_partially_signed_transaction_encoding } from "./test_partially_signed_transaction_encoding.js";
 
 /** @public */
 export function run_all_tests() {
@@ -38,4 +39,5 @@ export function run_all_tests() {
   run_one_test(test_orders);
   run_one_test(test_signed_transaction_intent);
   run_one_test(test_transaction_and_witness_encoding);
+  run_one_test(test_partially_signed_transaction_encoding);
 }

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

@@ -380,11 +380,11 @@ function issue_fungible_token_test() {
 }
 
 function issue_nft_test() {
-  const account_pubkey = make_default_account_privkey(
+  const account_privkey = make_default_account_privkey(
     MNEMONIC,
     Network.Testnet
   );
-  const receiving_privkey = make_receiving_address(account_pubkey, 0);
+  const receiving_privkey = make_receiving_address(account_privkey, 0);
   const receiving_pubkey = public_key_from_private_key(receiving_privkey);
 
   let encoded_nft = encode_output_issue_nft(

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

@@ -54,11 +54,11 @@ import {
 } from "./test_encode_other_outputs.js";
 
 export function test_htlc() {
-  const account_pubkey = make_default_account_privkey(
+  const account_privkey = make_default_account_privkey(
     MNEMONIC,
     Network.Testnet
   );
-  const receiving_privkey = make_receiving_address(account_pubkey, 0);
+  const receiving_privkey = make_receiving_address(account_privkey, 0);
 
   const htlc_coins_output = encode_output_htlc(
     Amount.from_atoms("40000"),