Offline signatures from SDK

.changelog/unreleased/SDK/4234-offline-sigs-in-sdk.md

@@ -0,0 +1,2 @@
+- Moved the signature generation logic for offline signing to the SDK.
+  ([\#4234](https://github.com/anoma/namada/pull/4234))

crates/apps_lib/src/cli.rs

@@ -296,7 +296,6 @@ pub mod cmds {
                 .subcommand(QueryEffNativeSupply::def().display_order(5))
                 .subcommand(QueryStakingRewardsRate::def().display_order(5))
                 // Actions
-                .subcommand(SignTx::def().display_order(6))
                 .subcommand(ShieldedSync::def().display_order(6))
                 .subcommand(GenIbcShieldingTransfer::def().display_order(6))
                 // Utils
@@ -1919,25 +1918,6 @@ pub mod cmds {
         }
     }
 
-    #[derive(Clone, Debug)]
-    pub struct SignTx(pub args::SignTx<args::CliTypes>);
-
-    impl SubCmd for SignTx {
-        const CMD: &'static str = "sign-tx";
-
-        fn parse(matches: &ArgMatches) -> Option<Self> {
-            matches
-                .subcommand_matches(Self::CMD)
-                .map(|matches| SignTx(args::SignTx::parse(matches)))
-        }
-
-        fn def() -> App {
-            App::new(Self::CMD)
-                .about(wrap!("Sign a transaction offline."))
-                .add_args::<args::SignTx<args::CliTypes>>()
-        }
-    }
-
     #[derive(Clone, Debug)]
     pub struct QueryValidatorState(
         pub args::QueryValidatorState<args::CliTypes>,
@@ -6761,47 +6741,6 @@ pub mod args {
         }
     }
 
-    impl CliToSdk<SignTx<SdkTypes>> for SignTx<CliTypes> {
-        type Error = std::io::Error;
-
-        fn to_sdk(
-            self,
-            ctx: &mut Context,
-        ) -> Result<SignTx<SdkTypes>, Self::Error> {
-            let tx = self.tx.to_sdk(ctx)?;
-            let tx_data = std::fs::read(self.tx_data)?;
-
-            Ok(SignTx::<SdkTypes> {
-                tx,
-                tx_data,
-                owner: ctx.borrow_chain_or_exit().get(&self.owner),
-            })
-        }
-    }
-
-    impl Args for SignTx<CliTypes> {
-        fn parse(matches: &ArgMatches) -> Self {
-            let tx = Tx::parse(matches);
-            let tx_path = TX_PATH.parse(matches);
-            let owner = OWNER.parse(matches);
-            Self {
-                tx,
-                tx_data: tx_path,
-                owner,
-            }
-        }
-
-        fn def(app: App) -> App {
-            app.add_args::<Tx<CliTypes>>()
-                .arg(TX_PATH.def().help(wrap!(
-                    "The path to the tx file with the serialized tx."
-                )))
-                .arg(
-                    OWNER.def().help(wrap!("The address of the account owner")),
-                )
-        }
-    }
-
     impl Args for ShieldedSync<CliTypes> {
         fn parse(matches: &ArgMatches) -> Self {
             let ledger_address = CONFIG_RPC_LEDGER_ADDRESS.parse(matches);

crates/apps_lib/src/client/utils.rs

@@ -8,17 +8,17 @@ use flate2::read::GzDecoder;
 use flate2::write::GzEncoder;
 use flate2::Compression;
 use itertools::Either;
-use namada_sdk::account::AccountPublicKeysMap;
 use namada_sdk::address::{Address, ImplicitAddress};
 use namada_sdk::args::DeviceTransport;
 use namada_sdk::borsh::BorshSerializeExt;
 use namada_sdk::chain::ChainId;
 use namada_sdk::dec::Dec;
 use namada_sdk::ibc::trace::ibc_token;
 use namada_sdk::key::*;
+use namada_sdk::signing::OfflineSignatures;
 use namada_sdk::string_encoding::StringEncoded;
 use namada_sdk::token;
-use namada_sdk::tx::{Authorization, Tx};
+use namada_sdk::tx::Tx;
 use namada_sdk::uint::Uint;
 use namada_sdk::wallet::{alias, LoadStoreError, Wallet};
 use namada_vm::validate_untrusted_wasm;
@@ -1035,7 +1035,7 @@ pub async fn sign_genesis_tx(
     }
 }
 
-/// Offline sign a transactions.
+/// Sign a transaction offline
 pub async fn sign_offline(
     args::SignOffline {
         tx_path,
@@ -1052,28 +1052,38 @@ pub async fn sign_offline(
         safe_exit(1)
     };
 
-    let mut tx =
-        if let Ok(transaction) = Tx::try_from_json_bytes(tx_data.as_ref()) {
-            transaction
-        } else {
-            eprintln!("Couldn't decode the transaction.");
-            safe_exit(1)
-        };
-
-    let account_public_keys_map = AccountPublicKeysMap::from_iter(
-        secret_keys.iter().map(|sk| sk.to_public()),
-    );
-
-    let signatures = tx.compute_section_signature(
-        &secret_keys,
-        &account_public_keys_map,
+    let tx = if let Ok(transaction) = Tx::try_from_json_bytes(tx_data.as_ref())
+    {
+        transaction
+    } else {
+        eprintln!("Couldn't decode the transaction.");
+        safe_exit(1)
+    };
+    let raw_header_hash = tx.raw_header_hash();
+    let header_hash = tx.header_hash();
+
+    let OfflineSignatures {
+        signatures,
+        wrapper_signature,
+    } = match namada_sdk::signing::generate_tx_signatures(
+        tx,
+        secret_keys,
         owner,
-    );
+        wrapper_signer,
+    )
+    .await
+    {
+        Ok(sigs) => sigs,
+        Err(e) => {
+            eprintln!("Couldn't generate offline signatures: {}", e);
+            safe_exit(1);
+        }
+    };
 
     for signature in &signatures {
         let filename = format!(
             "offline_signature_{}_{}.sig",
-            tx.raw_header_hash().to_string().to_lowercase(),
+            raw_header_hash.to_string().to_lowercase(),
             signature.pubkey,
         );
 
@@ -1094,42 +1104,24 @@ pub async fn sign_offline(
         );
     }
 
-    // Generate wrapper signature if requested
-    if let Some(wrapper_signer) = wrapper_signer {
-        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(),
-                None,
-            );
-
-            let filename = format!(
-                "offline_wrapper_signature_{}.sig",
-                tx.header_hash().to_string().to_lowercase()
-            );
-            let signature_path = match output_folder_path {
-                Some(ref path) => {
-                    path.join(filename).to_string_lossy().to_string()
-                }
-                None => filename,
-            };
+    // Dump wrapper signature if requested
+    if let Some(wrapper_signature) = wrapper_signature {
+        let filename = format!(
+            "offline_wrapper_signature_{}.sig",
+            header_hash.to_string().to_lowercase()
+        );
+        let signature_path = match output_folder_path {
+            Some(ref path) => path.join(filename).to_string_lossy().to_string(),
+            None => filename,
+        };
 
-            let signature_file = File::create(&signature_path)
-                .expect("Should be able to create signature file.");
+        let signature_file = File::create(&signature_path)
+            .expect("Should be able to create signature file.");
 
-            serde_json::to_writer_pretty(signature_file, &wrapper_signature)
-                .expect("Signature should be serializable.");
+        serde_json::to_writer_pretty(signature_file, &wrapper_signature)
+            .expect("Signature should be serializable.");
 
-            println!("Wrapper signature serialized at {}", signature_path);
-        } else {
-            eprintln!(
-                "A gas payer was provided but the transaction is not a wrapper"
-            );
-            safe_exit(1);
-        }
+        println!("Wrapper signature serialized at {}", signature_path);
     }
 }
 

crates/sdk/src/args.rs

@@ -2141,17 +2141,6 @@ impl TxReactivateValidator {
     }
 }
 
-#[derive(Clone, Debug)]
-/// Sign a transaction offline
-pub struct SignTx<C: NamadaTypes = SdkTypes> {
-    /// Common tx arguments
-    pub tx: Tx<C>,
-    /// Transaction data
-    pub tx_data: C::Data,
-    /// The account address
-    pub owner: C::Address,
-}
-
 #[derive(Clone, Debug)]
 /// Sync notes from MASP owned by the provided spending /
 /// viewing keys. Syncing can be told to stop at a given

crates/sdk/src/signing.rs

@@ -37,7 +37,7 @@ use namada_token::storage_key::balance_key;
 use namada_tx::data::pgf::UpdateStewardCommission;
 use namada_tx::data::pos::BecomeValidator;
 use namada_tx::data::{pos, Fee};
-use namada_tx::{MaspBuilder, Section, SignatureIndex, Tx};
+use namada_tx::{Authorization, MaspBuilder, Section, SignatureIndex, Tx};
 use rand::rngs::OsRng;
 use serde::{Deserialize, Serialize};
 use tokio::sync::RwLock;
@@ -435,6 +435,68 @@ pub async fn aux_signing_data(
     })
 }
 
+/// The transaction's signatures produced via the offline signing procedure
+pub struct OfflineSignatures {
+    /// Inner txs' signatures
+    pub signatures: Vec<SignatureIndex>,
+    /// Optional wrapper signature
+    pub wrapper_signature: Option<Authorization>,
+}
+
+/// Generates the transaction's signatures for offline signing purposes. This
+/// allows to sign both the inner transactions of the batch as well as the
+/// wrapper transaction and it supports multisignatures.
+///
+/// This function might need to modify the transaction by attaching the inner tx
+/// signatures to correctly produce the wrapper signature: since this change is
+/// only needed for the sake of this function and should not be propagated to
+/// the caller, this function consumes the actual tx.
+pub async fn generate_tx_signatures(
+    mut tx: Tx,
+    secret_keys: Vec<namada_core::key::common::SecretKey>,
+    owner: Option<Address>,
+    wrapper_signer: Option<namada_core::key::common::SecretKey>,
+) -> Result<OfflineSignatures, Error> {
+    let account_public_keys_map = AccountPublicKeysMap::from_iter(
+        secret_keys.iter().map(|sk| sk.to_public()),
+    );
+
+    let signatures = tx.compute_section_signature(
+        &secret_keys,
+        &account_public_keys_map,
+        owner,
+    );
+
+    // Generate wrapper signature if requested
+    let wrapper_signature = wrapper_signer
+        .map(|wrapper_signer| {
+            // Depends if we want to return an error or not?
+            if tx.header.wrapper().is_some() {
+                // Wrapper signature must be computed over the raw signatures
+                // too
+                tx.add_signatures(signatures.clone());
+                tx.protocol_filter();
+                Ok(Authorization::new(
+                    tx.sechashes(),
+                    [(0, wrapper_signer)].into_iter().collect(),
+                    None,
+                ))
+            } else {
+                Err(Error::Other(
+                    "A gas payer was provided but the transaction is not a \
+                     wrapper"
+                        .to_string(),
+                ))
+            }
+        })
+        .transpose()?;
+
+    Ok(OfflineSignatures {
+        signatures,
+        wrapper_signature,
+    })
+}
+
 /// Information about the post-fee balance of the tx's source. Used to correctly
 /// handle balance validation in the inner tx
 #[derive(Debug)]