comppose tx returns partially signed transaction and improve test

chainstate/src/rpc/mod.rs

@@ -65,7 +65,7 @@ trait ChainstateRpc {
 
     /// Returns the TxOutput for a specified UtxoOutPoint.
     #[method(name = "get_utxo")]
-    async fn utxo(&self, outpoint: UtxoOutPoint) -> RpcResult<Option<TxOutput>>;
+    async fn get_utxo(&self, outpoint: UtxoOutPoint) -> RpcResult<Option<TxOutput>>;
 
     /// Submit a block to be included in the chain
     #[method(name = "submit_block")]
@@ -191,7 +191,7 @@ impl ChainstateRpcServer for super::ChainstateHandle {
         Ok(blocks.into_iter().map(HexEncoded::new).collect())
     }
 
-    async fn utxo(&self, outpoint: UtxoOutPoint) -> RpcResult<Option<TxOutput>> {
+    async fn get_utxo(&self, outpoint: UtxoOutPoint) -> RpcResult<Option<TxOutput>> {
         rpc::handle_result(
             self.call_mut(move |this| {
                 this.utxo(&outpoint).map(|utxo| utxo.map(|utxo| utxo.take_output()))

test/functional/test_framework/wallet_cli_controller.py

@@ -24,8 +24,8 @@
 
 from typing import Optional, List, Tuple, Union
 
-ONE_MB = 2**20
-READ_TIMEOUT_SEC = 30
+TEN_MB = 100*2**20
+READ_TIMEOUT_SEC = 3
 DEFAULT_ACCOUNT_INDEX = 0
 
 @dataclass
@@ -96,14 +96,23 @@ async def __aexit__(self, exc_type, exc_value, traceback):
         self.wallet_commands_file.close()
 
     async def _read_available_output(self) -> str:
+        result = ''
+        output = ''
+        num_tries = 0
         try:
-            output = await asyncio.wait_for(self.process.stdout.read(ONE_MB), timeout=READ_TIMEOUT_SEC)
-            self.wallet_commands_file.write(output)
-            result = output.decode().strip()
+            while not result and num_tries < 3:
+                output = await asyncio.wait_for(self.process.stdout.read(TEN_MB), timeout=READ_TIMEOUT_SEC)
+                self.wallet_commands_file.write(output)
+                self.log.info(f"read result '{output}' {not output} {output == ''}")
+                num_tries = num_tries + 1
+                if not output:
+                    continue
+                result = output.decode().strip()
+                self.log.info(f"read result '{result}' {not result} {result == ''}")
 
             try:
                 while True:
-                    output = await asyncio.wait_for(self.process.stdout.read(ONE_MB), timeout=0.1)
+                    output = await asyncio.wait_for(self.process.stdout.read(TEN_MB), timeout=0.1)
                     if not output:
                         break
                     self.wallet_commands_file.write(output)
@@ -112,7 +121,8 @@ async def _read_available_output(self) -> str:
                 pass
 
             return result
-        except:
+        except Exception as e:
+            self.log.error(f"read tiemout '{e}' {output} {output == ''}")
             self.wallet_commands_file.write(b"read from stdout timedout\n")
             return ''
 
@@ -205,7 +215,7 @@ async def send_to_address(self, address: str, amount: int, selected_utxos: List[
         return await self._write_command(f"address-send {address} {amount} {' '.join(map(str, selected_utxos))}\n")
 
     async def compose_transaction(self, outputs: List[TxOutput], selected_utxos: List[UtxoOutpoint]) -> str:
-        return await self._write_command(f"transaction-compose {' '.join(map(str, outputs))} --utxos {' '.join(map(str, selected_utxos))}\n")
+        return await self._write_command(f"transaction-compose {' '.join(map(str, outputs))} --utxos {' --utxos '.join(map(str, selected_utxos))}\n")
 
     async def send_tokens_to_address(self, token_id: str, address: str, amount: Union[float, str]):
         return await self._write_command(f"token-send {token_id} {address} {amount}\n")

test/functional/wallet_tx_compose.py

@@ -87,21 +87,25 @@ async def async_test(self):
             self.log.info(f"best block height = {best_block_height}")
             assert_equal(best_block_height, '0')
 
-            # new address
-            pub_key_bytes = await wallet.new_public_key()
-            assert_equal(len(pub_key_bytes), 33)
-
             # Get chain tip
             tip_id = node.chainstate_best_block_id()
-            genesis_block_id = tip_id
             self.log.debug(f'Tip: {tip_id}')
 
+            coins_to_send = random.randint(2, 10)
+            # new address
+            addresses = []
+            num_utxos = random.randint(1, 3)
+            for _ in range(num_utxos):
+                pub_key_bytes = await wallet.new_public_key()
+                assert_equal(len(pub_key_bytes), 33)
+                addresses.append(pub_key_bytes)
+
             # Submit a valid transaction
-            coins_to_send = random.randint(2, 100)
-            output = {
-                    'Transfer': [ { 'Coin': coins_to_send * ATOMS_PER_COIN }, { 'PublicKey': {'key': {'Secp256k1Schnorr' : {'pubkey_data': pub_key_bytes}}} } ],
-            }
-            encoded_tx, tx_id = make_tx([reward_input(tip_id)], [output], 0)
+            def make_output(pub_key_bytes):
+                return {
+                        'Transfer': [ { 'Coin': coins_to_send * ATOMS_PER_COIN }, { 'PublicKey': {'key': {'Secp256k1Schnorr' : {'pubkey_data': pub_key_bytes}}} } ],
+                }
+            encoded_tx, tx_id = make_tx([reward_input(tip_id)], [make_output(pk) for pk in addresses], 0)
 
             self.log.debug(f"Encoded transaction {tx_id}: {encoded_tx}")
 
@@ -116,7 +120,7 @@ async def async_test(self):
             # sync the wallet
             assert_in("Success", await wallet.sync())
 
-            assert_in(f"Coins amount: {coins_to_send}", await wallet.get_balance())
+            assert_in(f"Coins amount: {coins_to_send * len(addresses)}", await wallet.get_balance())
 
             ## create a new account and get an address
             await wallet.create_new_account()
@@ -126,20 +130,23 @@ async def async_test(self):
 
             change_address = await wallet.new_address()
             # transfer all except 1 coin to the new acc, and add 0.1 fee
-            outputs = [ TxOutput(acc1_address, str(coins_to_send - 1)), TxOutput(change_address, "0.9") ]
+            num_outputs = random.randint(0, len(addresses) - 1)
+            outputs = [TxOutput(acc1_address, str(coins_to_send)) for _ in range(num_outputs)] + [ TxOutput(acc1_address, str(coins_to_send - 1)), TxOutput(change_address, "0.9") ]
 
-            # check we have 1 unspent utxo
+            # check we have unspent utxos
             utxos = await wallet.list_utxos()
-            assert_equal(len(utxos), 1)
+            assert_equal(len(utxos), len(addresses))
 
-            # compose a transaction with that utxo and 2 outputs
+            # compose a transaction with all our utxos and n outputs to the other acc and 1 as change
             output = await wallet.compose_transaction(outputs, utxos)
+            self.log.info(f"compose output: '{output}'")
             assert_in("The hex encoded transaction is", output)
-            # check the fees include the 0.1
-            assert_in(f"Coins amount: 0.1", output)
+            # check the fees include the 0.1 + any extra utxos
+            assert_in(f"Coins amount: {((len(addresses) - (num_outputs + 1))*coins_to_send)}.1", output)
             encoded_tx = output.split('\n')[1]
 
             output = await wallet.sign_raw_transaction(encoded_tx)
+            self.log.info(f"sign output: '{output}'")
             assert_in("The transaction has been fully signed signed", output)
             signed_tx = output.split('\n')[2]
 
@@ -150,9 +157,14 @@ async def async_test(self):
             self.generate_block()
 
             assert_in("Success", await wallet.sync())
+            # check we have the change
             assert_in(f"Coins amount: 0.9", await wallet.get_balance())
+            # and 1 new utxo
+            assert_equal(1, len(await wallet.list_utxos()))
+
             await wallet.select_account(1)
-            assert_in(f"Coins amount: {coins_to_send-1}", await wallet.get_balance())
+            assert_in(f"Coins amount: {num_outputs * coins_to_send + coins_to_send-1}", await wallet.get_balance())
+            assert_equal(num_outputs + 1, len(await wallet.list_utxos()))
 
 
 if __name__ == '__main__':

wallet/wallet-cli-lib/src/commands/helper_types.rs

@@ -222,10 +222,14 @@ pub fn parse_output<N: NodeInterface>(
 
     let dest = Address::from_str(chain_config, parts[1])
         .and_then(|addr| addr.decode_object(chain_config))
-        .map_err(|err| WalletCliError::<N>::InvalidInput(err.to_string()))?;
+        .map_err(|err| {
+            WalletCliError::<N>::InvalidInput(format!("invalid address {} {err}", parts[1]))
+        })?;
 
     let amount = DecimalAmount::from_str(parts[2])
-        .map_err(|err| WalletCliError::<N>::InvalidInput(err.to_string()))?
+        .map_err(|err| {
+            WalletCliError::<N>::InvalidInput(format!("invalid amount {} {err}", parts[2]))
+        })?
         .to_amount(chain_config.coin_decimals())
         .ok_or(WalletCliError::<N>::InvalidInput(
             "invalid coins amount".to_string(),
@@ -235,7 +239,7 @@ pub fn parse_output<N: NodeInterface>(
         "transfer" => TxOutput::Transfer(OutputValue::Coin(amount), dest),
         _ => {
             return Err(WalletCliError::<N>::InvalidInput(
-                "Invalid input: unknown ID type".into(),
+                "Invalid output: unknown type".into(),
             ));
         }
     };

wallet/wallet-cli-lib/src/commands/mod.rs

@@ -611,8 +611,13 @@ pub enum WalletCommand {
     #[clap(hide = true)]
     GenerateBlocks { block_count: u32 },
 
-    /// Send a given coin amount to a given address. The wallet will automatically calculate the required information
-    /// Optionally, one can also mention the utxos to be used.
+    /// Compose a new transaction from the specified outputs and selected utxos
+    /// The transaction is returned in a hex encoded form that can be passed to account-sign-raw-transaction
+    /// and also prints the fees that will be paied by the transaction
+    /// example usage:
+    /// transaction-compose transfer(tmt1q8lhgxhycm8e6yk9zpnetdwtn03h73z70c3ha4l7,0.9) --utxos
+    ///   tx(000000000000000000059fa50103b9683e51e5aba83b8a34c9b98ce67d66136c,1)
+    /// which creates a transaction with 1 output and 1 input
     #[clap(name = "transaction-compose")]
     TransactionCompose {
         /// The transaction outputs, in the format `transfer(address,amount)`
@@ -1027,16 +1032,23 @@ where
                     Ok(signed_tx) => {
                         let result_hex: HexEncoded<SignedTransaction> = signed_tx.into();
 
-                        let qr_code = utils::qrcode::qrcode_from_str(result_hex.to_string())
-                            .map_err(WalletCliError::QrCodeEncoding)?;
-                        let qr_code_string = qr_code.encode_to_console_string_with_defaults(1);
+                        let qr_code_string = utils::qrcode::qrcode_from_str(result_hex.to_string())
+                            .map(|qr_code| qr_code.encode_to_console_string_with_defaults(1));
 
-                        format!(
+                        match qr_code_string {
+                            Ok(qr_code_string) => format!(
                             "The transaction has been fully signed signed as is ready to be broadcast to network. \
                              You can use the command `node-submit-transaction` in a wallet connected to the internet (this one or elsewhere). \
                              Pass the following data to the wallet to broadcast:\n\n{result_hex}\n\n\
                              Or scan the Qr code with it:\n\n{qr_code_string}"
-                        )
+                        ),
+                            Err(_) => format!(
+                            "The transaction has been fully signed signed as is ready to be broadcast to network. \
+                             You can use the command `node-submit-transaction` in a wallet connected to the internet (this one or elsewhere). \
+                             Pass the following data to the wallet to broadcast:\n\n{result_hex}\n\n\
+                             Transaction is too long to be put into a Qr code"
+                        ),
+                        }
                     }
                     Err(WalletError::FailedToConvertPartiallySignedTx(partially_signed_tx)) => {
                         let result_hex: HexEncoded<PartiallySignedTransaction> =
@@ -1204,6 +1216,8 @@ where
             }
 
             WalletCommand::TransactionCompose { outputs, utxos } => {
+                eprintln!("outputs: {outputs:?}");
+                eprintln!("utxos: {utxos:?}");
                 let outputs: Vec<TxOutput> = outputs
                     .into_iter()
                     .map(|input| parse_output(input, chain_config))

wallet/wallet-controller/src/lib.rs

@@ -65,7 +65,11 @@ pub use node_comm::{
     rpc_client::NodeRpcClient,
 };
 use wallet::{
-    account::currency_grouper::{self, Currency},
+    account::{
+        currency_grouper::{self, Currency},
+        PartiallySignedTransaction,
+    },
+    get_tx_output_destination,
     wallet::WalletPoolsFilter,
     wallet_events::WalletEvents,
     DefaultWallet, WalletError, WalletResult,
@@ -613,24 +617,43 @@ impl<T: NodeInterface + Clone + Send + Sync + 'static, W: WalletEvents> Controll
         &self,
         inputs: Vec<UtxoOutPoint>,
         outputs: Vec<TxOutput>,
-    ) -> Result<(Transaction, Balances), ControllerError<T>> {
-        let fees = self.get_fees(&inputs, &outputs).await?;
+    ) -> Result<(PartiallySignedTransaction, Balances), ControllerError<T>> {
+        let input_utxos = self.fetch_utxos(&inputs).await?;
+        let fees = self.get_fees(&input_utxos, &outputs)?;
         let fees = into_balances(&self.rpc_client, &self.chain_config, fees).await?;
 
+        let num_inputs = inputs.len();
         let inputs = inputs.into_iter().map(TxInput::Utxo).collect();
 
         let tx = Transaction::new(0, inputs, outputs)
             .map_err(|err| ControllerError::WalletError(WalletError::TransactionCreation(err)))?;
 
+        let destinations = input_utxos
+            .iter()
+            .map(|txo| {
+                get_tx_output_destination(txo, &|_| None)
+                    .ok_or_else(|| WalletError::UnsupportedTransactionOutput(Box::new(txo.clone())))
+            })
+            .collect::<Result<Vec<_>, WalletError>>()
+            .map_err(ControllerError::WalletError)?;
+
+        let tx = PartiallySignedTransaction::new(
+            tx,
+            vec![None; num_inputs],
+            input_utxos.into_iter().map(Option::Some).collect(),
+            destinations.into_iter().map(Option::Some).collect(),
+        )
+        .map_err(ControllerError::WalletError)?;
+
         Ok((tx, fees))
     }
 
-    async fn get_fees(
+    fn get_fees(
         &self,
-        inputs: &[UtxoOutPoint],
+        inputs: &[TxOutput],
         outputs: &[TxOutput],
     ) -> Result<BTreeMap<Currency, Amount>, ControllerError<T>> {
-        let mut inputs = self.fetch_and_group_inputs(inputs).await?;
+        let mut inputs = self.group_inputs(inputs)?;
         let outputs = self.group_outpus(outputs)?;
 
         let mut fees = BTreeMap::new();
@@ -646,7 +669,7 @@ impl<T: NodeInterface + Clone + Send + Sync + 'static, W: WalletEvents> Controll
             fees.insert(currency, fee);
         }
         // add any leftover inputs
-        fees.extend(inputs.into_iter());
+        fees.extend(inputs);
         Ok(fees)
     }
 
@@ -669,15 +692,12 @@ impl<T: NodeInterface + Clone + Send + Sync + 'static, W: WalletEvents> Controll
         .map_err(|err| ControllerError::WalletError(err))
     }
 
-    async fn fetch_and_group_inputs(
+    fn group_inputs(
         &self,
-        inputs: &[UtxoOutPoint],
+        input_utxos: &[TxOutput],
     ) -> Result<BTreeMap<Currency, Amount>, ControllerError<T>> {
-        let tasks: FuturesUnordered<_> =
-            inputs.iter().map(|input| self.fetch_utxo(input)).collect();
-        let input_utxos: Vec<TxOutput> = tasks.try_collect().await?;
         currency_grouper::group_utxos_for_input(
-            input_utxos.into_iter(),
+            input_utxos.iter(),
             |tx_output| tx_output,
             |total: &mut Amount, _, amount| -> Result<(), WalletError> {
                 *total = (*total + amount).ok_or(WalletError::OutputAmountOverflow)?;
@@ -688,6 +708,16 @@ impl<T: NodeInterface + Clone + Send + Sync + 'static, W: WalletEvents> Controll
         .map_err(|err| ControllerError::WalletError(err))
     }
 
+    async fn fetch_utxos(
+        &self,
+        inputs: &[UtxoOutPoint],
+    ) -> Result<Vec<TxOutput>, ControllerError<T>> {
+        let tasks: FuturesUnordered<_> =
+            inputs.iter().map(|input| self.fetch_utxo(input)).collect();
+        let input_utxos: Vec<TxOutput> = tasks.try_collect().await?;
+        Ok(input_utxos)
+    }
+
     async fn fetch_utxo(&self, input: &UtxoOutPoint) -> Result<TxOutput, ControllerError<T>> {
         let utxo = self
             .rpc_client

wallet/wallet-node-client/src/rpc_client/client_impl.rs

@@ -273,7 +273,7 @@ impl NodeInterface for NodeRpcClient {
     }
 
     async fn get_utxo(&self, outpoint: UtxoOutPoint) -> Result<Option<TxOutput>, Self::Error> {
-        ChainstateRpcClient::utxo(&self.http_client, outpoint)
+        ChainstateRpcClient::get_utxo(&self.http_client, outpoint)
             .await
             .map_err(NodeRpcError::ResponseError)
     }

wallet/wallet-rpc-lib/src/rpc/mod.rs

@@ -695,7 +695,7 @@ impl<N: NodeInterface + Clone + Send + Sync + 'static> WalletRpc<N> {
         &self,
         inputs: Vec<UtxoOutPoint>,
         outputs: Vec<TxOutput>,
-    ) -> WRpcResult<(Transaction, Balances), N> {
+    ) -> WRpcResult<(PartiallySignedTransaction, Balances), N> {
         self.wallet
             .call_async(move |w| {
                 Box::pin(async move { w.compose_transaction(inputs, outputs).await })