Merge pull request #4120 from anoma/grarco/fix-offline-wrapper-sig

Fix offline wrapper signature by elsewho

.changelog/unreleased/bug-fixes/4120-fix-offline-wrapper-sig.md

@@ -0,0 +1,3 @@
+- Fixed a bug that was preventing generating wrapper signatures
+  offline. Removed a redundant `sign-tx` client command.
+  ([\#4120](https://github.com/anoma/namada/pull/4120))

crates/apps_lib/src/cli.rs

@@ -391,7 +391,6 @@ pub mod cmds {
             let query_metadata = Self::parse_with_ctx(matches, QueryMetaData);
             let add_to_eth_bridge_pool =
                 Self::parse_with_ctx(matches, AddToEthBridgePool);
-            let sign_tx = Self::parse_with_ctx(matches, SignTx);
             let shielded_sync = Self::parse_with_ctx(matches, ShieldedSync);
             let gen_ibc_shielding =
                 Self::parse_with_ctx(matches, GenIbcShieldingTransfer);
@@ -451,7 +450,6 @@ pub mod cmds {
                 .or(query_native_supply)
                 .or(query_staking_rewards_rate)
                 .or(query_account)
-                .or(sign_tx)
                 .or(shielded_sync)
                 .or(gen_ibc_shielding)
                 .or(utils)
@@ -546,7 +544,6 @@ pub mod cmds {
         QueryPgf(QueryPgf),
         QueryValidatorState(QueryValidatorState),
         QueryRewards(QueryRewards),
-        SignTx(SignTx),
         ShieldedSync(ShieldedSync),
         GenIbcShieldingTransfer(GenIbcShieldingTransfer),
     }
@@ -7573,7 +7570,6 @@ pub mod args {
                         "The file path containing a serialized signature of \
                          the entire transaction for gas payment."
                     ))
-                    .requires(SIGNATURES.name)
                     .conflicts_with(FEE_PAYER_OPT.name),
             )
             .arg(OUTPUT_FOLDER_PATH.def().help(wrap!(

crates/apps_lib/src/cli/client.rs

@@ -796,18 +796,6 @@ impl CliApi {
                         let namada = ctx.to_sdk(client, io);
                         rpc::query_account(&namada, args).await;
                     }
-                    Sub::SignTx(SignTx(args)) => {
-                        let chain_ctx = ctx.borrow_mut_chain_or_exit();
-                        let ledger_address =
-                            chain_ctx.get(&args.tx.ledger_address);
-                        let client = client.unwrap_or_else(|| {
-                            C::from_tendermint_address(&ledger_address)
-                        });
-                        client.wait_until_node_is_synced(&io).await?;
-                        let args = args.to_sdk(&mut ctx)?;
-                        let namada = ctx.to_sdk(client, io);
-                        tx::sign_tx(&namada, args).await?;
-                    }
                 }
             }
             cli::NamadaClient::WithoutContext(cmd_box) => {

crates/apps_lib/src/client/tx.rs

@@ -283,9 +283,27 @@ pub async fn submit_custom<N: Namada>(
 where
     <N::Client as namada_sdk::io::Client>::Error: std::fmt::Display,
 {
-    let (tx, signing_data) = args.build(namada).await?;
+    let (mut tx, signing_data) = args.build(namada).await?;
+
+    if args.tx.dump_tx {
+        return tx::dump_tx(namada.io(), &args.tx, tx);
+    }
+    if args.tx.dump_wrapper_tx {
+        // Attach the provided inner signatures to the tx (if any)
+        let signatures = args
+            .tx
+            .signatures
+            .iter()
+            .map(|bytes| {
+                tx::SignatureIndex::try_from_json_bytes(bytes).map_err(|err| {
+                    error::Error::Encode(error::EncodingError::Serde(
+                        err.to_string(),
+                    ))
+                })
+            })
+            .collect::<error::Result<Vec<_>>>()?;
+        tx.add_signatures(signatures);
 
-    if args.tx.dump_tx || args.tx.dump_wrapper_tx {
         return tx::dump_tx(namada.io(), &args.tx, tx);
     }
 
@@ -1139,82 +1157,6 @@ where
     Ok(())
 }
 
-pub async fn sign_tx<N: Namada>(
-    namada: &N,
-    args::SignTx {
-        tx: tx_args,
-        tx_data,
-        owner,
-    }: args::SignTx,
-) -> Result<(), error::Error>
-where
-    <N::Client as namada_sdk::io::Client>::Error: std::fmt::Display,
-{
-    let tx = if let Ok(transaction) = Tx::try_from_json_bytes(tx_data.as_ref())
-    {
-        transaction
-    } else {
-        edisplay_line!(namada.io(), "Couldn't decode the transaction.");
-        return Err(error::Error::Other(
-            "Couldn't decode the transaction.".to_string(),
-        ));
-    };
-
-    let default_signer = Some(owner.clone());
-    let signing_data = aux_signing_data(
-        namada,
-        &tx_args,
-        Some(owner.clone()),
-        default_signer,
-        false,
-    )
-    .await?;
-
-    let mut wallet = namada.wallet_mut().await;
-
-    let mut secret_keys = vec![];
-    for public_key in &signing_data.public_keys {
-        let secret_key =
-            signing::find_key_by_pk(&mut wallet, &tx_args, public_key)?;
-        secret_keys.push(secret_key);
-    }
-
-    if let Some(account_public_keys_map) = signing_data.account_public_keys_map
-    {
-        let signatures = tx.compute_section_signature(
-            &secret_keys,
-            &account_public_keys_map,
-            Some(owner),
-        );
-
-        for signature in &signatures {
-            let filename = format!(
-                "offline_signature_{}_{}.tx",
-                tx.raw_header_hash(),
-                signature.pubkey,
-            );
-            let output_path = match &tx_args.output_folder {
-                Some(path) => path.join(filename),
-                None => filename.into(),
-            };
-            let signature_path = File::create(&output_path)
-                .expect("Should be able to create signature file.");
-            signature.to_writer_json(signature_path).expect(
-                "Signature should be serializable and the file writeable.",
-            );
-
-            display_line!(
-                namada.io(),
-                "Signature for {} serialized at {}",
-                signature.pubkey,
-                output_path.display()
-            );
-        }
-    }
-
-    Ok(())
-}
-
 pub async fn submit_reveal_pk<N: Namada>(
     namada: &N,
     args: args::RevealPk,

crates/apps_lib/src/client/utils.rs

@@ -1099,6 +1099,7 @@ pub async fn sign_offline(
         if tx.header.wrapper().is_some() {
             // Wrapper signature must be computed over the raw signatures too
             tx.add_signatures(signatures);
+            tx.protocol_filter();
             let wrapper_signature = Authorization::new(
                 tx.sechashes(),
                 [(0, wrapper_signer)].into_iter().collect(),
@@ -1124,10 +1125,10 @@ pub async fn sign_offline(
 
             println!("Wrapper signature serialized at {}", signature_path);
         } else {
-            println!(
-                "A gas payer was provided but the transaction is not a \
-                 wrapper: skipping the wrapper signature"
+            eprintln!(
+                "A gas payer was provided but the transaction is not a wrapper"
             );
+            safe_exit(1);
         }
     }
 }

crates/sdk/src/tx.rs

@@ -3757,8 +3757,15 @@ pub async fn build_custom(
     let signing_data = if let Some(wrapper_signature) =
         &tx_args.wrapper_signature
     {
+        if tx.header.wrapper().is_none() {
+            return Err(Error::Other(
+                "A wrapper signature was provided but the transaction is not \
+                 a wrapper"
+                    .to_string(),
+            ));
+        }
         // Attach the provided signatures to the tx without the need to produce
-        // any mroe signatures
+        // any more signatures
         let signatures = tx_args.signatures.iter().try_fold(
             vec![],
             |mut acc, bytes| -> Result<Vec<_>> {