Merge of #3715

.changelog/unreleased/improvements/3715-improve-offline-tx-handling.md

@@ -0,0 +1,3 @@
+- Improve the format of dumped txs. Added  command to
+  generate signature without requiring a network connection.
+  ([\#3715](https://github.com/anoma/namada/pull/3715))

.github/workflows/scripts/e2e.json

@@ -30,4 +30,4 @@
     "e2e::wallet_tests::wallet_encrypted_key_cmds_env_var": 1,
     "e2e::wallet_tests::wallet_unencrypted_key_cmds": 1,
     "e2e::ledger_tests::masp_txs_and_queries": 82
-}
+}

crates/apps_lib/src/cli.rs

@@ -33,6 +33,7 @@ const WALLET_CMD: &str = "wallet";
 const RELAYER_CMD: &str = "relayer";
 
 pub mod cmds {
+    use super::args::CliTypes;
     use super::utils::*;
     use super::{
         args, ArgMatches, CLIENT_CMD, NODE_CMD, RELAYER_CMD, WALLET_CMD,
@@ -2458,6 +2459,7 @@ pub mod cmds {
         InitGenesisEstablishedAccount(InitGenesisEstablishedAccount),
         InitGenesisValidator(InitGenesisValidator),
         PkToTmAddress(PkToTmAddress),
+        SignOffline(SignOffline),
         DefaultBaseDir(DefaultBaseDir),
         EpochSleep(EpochSleep),
         ValidateGenesisTemplates(ValidateGenesisTemplates),
@@ -2486,6 +2488,8 @@ pub mod cmds {
                     SubCmd::parse(matches).map(Self::InitGenesisValidator);
                 let pk_to_tm_address =
                     SubCmd::parse(matches).map(Self::PkToTmAddress);
+                let sign_offline =
+                    SubCmd::parse(matches).map(Self::SignOffline);
                 let default_base_dir =
                     SubCmd::parse(matches).map(Self::DefaultBaseDir);
                 let epoch_sleep = SubCmd::parse(matches).map(Self::EpochSleep);
@@ -2508,6 +2512,7 @@ pub mod cmds {
                     .or(validate_genesis_templates)
                     .or(genesis_tx)
                     .or(parse_migrations_json)
+                    .or(sign_offline)
             })
         }
 
@@ -2522,6 +2527,7 @@ pub mod cmds {
                 .subcommand(InitGenesisEstablishedAccount::def())
                 .subcommand(InitGenesisValidator::def())
                 .subcommand(PkToTmAddress::def())
+                .subcommand(SignOffline::def())
                 .subcommand(DefaultBaseDir::def())
                 .subcommand(EpochSleep::def())
                 .subcommand(ValidateGenesisTemplates::def())
@@ -3189,6 +3195,25 @@ pub mod cmds {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct SignOffline(pub args::SignOffline<CliTypes>);
+
+    impl SubCmd for SignOffline {
+        const CMD: &'static str = "sign-offline";
+
+        fn parse(matches: &ArgMatches) -> Option<Self> {
+            matches.subcommand_matches(Self::CMD).map(|matches| {
+                Self(args::SignOffline::<CliTypes>::parse(matches))
+            })
+        }
+
+        fn def() -> App {
+            App::new(Self::CMD)
+                .about(wrap!("Offlne sign a transaction."))
+                .add_args::<args::SignOffline<CliTypes>>()
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct DefaultBaseDir(pub args::DefaultBaseDir);
 
@@ -3444,6 +3469,8 @@ pub mod args {
         DefaultFn(|| PortId::from_str("transfer").unwrap()),
     );
     pub const PRE_GENESIS: ArgFlag = flag("pre-genesis");
+    pub const PRIVATE_KEYS: ArgMulti<WalletKeypair, GlobStar> =
+        arg_multi("secret-keys");
     pub const PROPOSAL_PGF_STEWARD: ArgFlag = flag("pgf-stewards");
     pub const PROPOSAL_PGF_FUNDING: ArgFlag = flag("pgf-funding");
     pub const PROTOCOL_KEY: ArgOpt<WalletPublicKey> = arg_opt("protocol-key");
@@ -8040,6 +8067,70 @@ pub mod args {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct SignOffline<C: NamadaTypes = SdkTypes> {
+        pub tx_path: PathBuf,
+        pub secret_keys: Vec<C::Keypair>,
+        pub owner: C::Address,
+        pub output_folder_path: Option<PathBuf>,
+    }
+
+    impl Args for SignOffline<CliTypes> {
+        fn parse(matches: &ArgMatches) -> Self {
+            let tx_path = DATA_PATH.parse(matches);
+            let secret_keys = PRIVATE_KEYS.parse(matches);
+            let owner = OWNER.parse(matches);
+            let output_folder_path = OUTPUT_FOLDER_PATH.parse(matches);
+
+            Self {
+                tx_path,
+                secret_keys,
+                owner,
+                output_folder_path,
+            }
+        }
+
+        fn def(app: App) -> App {
+            app.arg(
+                DATA_PATH
+                    .def()
+                    .help(wrap!("The path to the serialized transaction.")),
+            )
+            .arg(PRIVATE_KEYS.def().help(wrap!(
+                "The set of private keys to use to sign the transaction. The \
+                 order matters."
+            )))
+            .arg(OWNER.def().help(wrap!("The owner's address.")))
+            .arg(
+                OUTPUT_FOLDER_PATH
+                    .def()
+                    .help("Folder to where serialize the signatures"),
+            )
+        }
+    }
+
+    impl CliToSdk<SignOffline<SdkTypes>> for SignOffline<CliTypes> {
+        type Error = std::io::Error;
+
+        fn to_sdk(
+            self,
+            ctx: &mut Context,
+        ) -> Result<SignOffline<SdkTypes>, Self::Error> {
+            let chain_ctx = ctx.borrow_mut_chain_or_exit();
+
+            Ok(SignOffline::<SdkTypes> {
+                tx_path: self.tx_path,
+                secret_keys: self
+                    .secret_keys
+                    .iter()
+                    .map(|key| chain_ctx.get_cached(key))
+                    .collect(),
+                owner: chain_ctx.get(&self.owner),
+                output_folder_path: self.output_folder_path,
+            })
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct DefaultBaseDir {}
 

crates/apps_lib/src/cli/client.rs

@@ -816,6 +816,12 @@ impl CliApi {
                     ClientUtils::PkToTmAddress(PkToTmAddress(args)) => {
                         utils::pk_to_tm_address(global_args, args)
                     }
+                    ClientUtils::SignOffline(SignOffline(args)) => {
+                        let mut ctx = cli::Context::new::<IO>(global_args)
+                            .expect("expected to construct a context");
+                        let args = args.to_sdk(&mut ctx)?;
+                        utils::sign_offline(args).await
+                    }
                     ClientUtils::DefaultBaseDir(DefaultBaseDir(args)) => {
                         utils::default_base_dir(global_args, args)
                     }

crates/apps_lib/src/client/tx.rs

@@ -1071,8 +1071,11 @@ where
         transaction
     } else {
         edisplay_line!(namada.io(), "Couldn't decode the transaction.");
-        safe_exit(1)
+        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,
@@ -1110,15 +1113,11 @@ where
                 Some(path) => path.join(filename),
                 None => filename.into(),
             };
-
             let signature_path = File::create(&output_path)
                 .expect("Should be able to create signature file.");
+            serde_json::to_writer_pretty(signature_path, &signature)
+                .expect("Signature should be serializable.");
 
-            serde_json::to_writer_pretty(
-                signature_path,
-                &signature.serialize(),
-            )
-            .expect("Signature should be deserializable.");
             display_line!(
                 namada.io(),
                 "Signature for {} serialized at {}",
@@ -1127,6 +1126,7 @@ where
             );
         }
     }
+
     Ok(())
 }
 

crates/apps_lib/src/client/utils.rs

@@ -9,13 +9,15 @@ use flate2::read::GzDecoder;
 use flate2::write::GzEncoder;
 use flate2::Compression;
 use itertools::Either;
+use namada_sdk::account::AccountPublicKeysMap;
 use namada_sdk::address::Address;
 use namada_sdk::args::DeviceTransport;
 use namada_sdk::chain::ChainId;
 use namada_sdk::dec::Dec;
 use namada_sdk::key::*;
 use namada_sdk::string_encoding::StringEncoded;
 use namada_sdk::token;
+use namada_sdk::tx::Tx;
 use namada_sdk::uint::Uint;
 use namada_sdk::wallet::{alias, LoadStoreError, Wallet};
 use namada_vm::validate_untrusted_wasm;
@@ -1031,6 +1033,64 @@ pub async fn sign_genesis_tx(
     }
 }
 
+/// Offline sign a transactions.
+pub async fn sign_offline(
+    args::SignOffline {
+        tx_path,
+        secret_keys,
+        owner,
+        output_folder_path,
+    }: args::SignOffline,
+) {
+    let tx_data = if let Ok(tx_data) = fs::read(&tx_path) {
+        tx_data
+    } else {
+        eprintln!("Couldn't open file at {}", tx_path.display());
+        safe_exit(1)
+    };
+
+    let tx = if let Ok(transaction) = Tx::deserialize(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,
+        Some(owner),
+    );
+
+    for signature in &signatures {
+        let filename = format!(
+            "offline_signature_{}_{}.sig",
+            tx.header_hash().to_string().to_lowercase(),
+            signature.pubkey,
+        );
+
+        let tx_path = match output_folder_path {
+            Some(ref path) => path.join(filename).to_string_lossy().to_string(),
+            None => filename,
+        };
+
+        let signature_path = File::create(&tx_path)
+            .expect("Should be able to create signature file.");
+
+        serde_json::to_writer_pretty(signature_path, &signature)
+            .expect("Signature should be deserializable.");
+
+        println!(
+            "Signature for {} serialized at {}",
+            signature.pubkey, tx_path
+        );
+    }
+}
+
 /// Add a spinning wheel to a message for long running commands.
 /// Can be turned off for E2E tests by setting the `REDUCED_CLI_PRINTING`
 /// environment variable.

crates/core/src/hash.rs

@@ -10,7 +10,7 @@ use data_encoding::HEXUPPER;
 use namada_macros::BorshDeserializer;
 #[cfg(feature = "migrations")]
 use namada_migrations::*;
-use serde::{Deserialize, Serialize};
+use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
 use sha2::{Digest, Sha256};
 use thiserror::Error;
 
@@ -51,12 +51,53 @@ pub type HashResult<T> = std::result::Result<T, Error>;
     BorshDeserialize,
     BorshDeserializer,
     BorshSchema,
-    Serialize,
-    Deserialize,
 )]
 /// A hash, typically a sha-2 hash of a tx
 pub struct Hash(pub [u8; HASH_LENGTH]);
 
+impl Serialize for Hash {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        // Convert the byte array to a hex string
+        let hex_string = HEXUPPER.encode(&self.0);
+        // Serialize the hex string
+        serializer.serialize_str(&hex_string)
+    }
+}
+
+impl<'de> Deserialize<'de> for Hash {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        // Deserialize a hex string
+        let hex_string =
+            <String as serde::Deserialize>::deserialize(deserializer)?;
+        // Convert the hex string back to a byte array
+        let bytes = HEXUPPER
+            .decode(hex_string.as_bytes())
+            .map_err(de::Error::custom)?;
+
+        // Ensure the byte array has the correct length
+        if bytes.len() != HASH_LENGTH {
+            return Err(de::Error::custom(format!(
+                "Invalid length: expected {} bytes, got {}",
+                HASH_LENGTH,
+                bytes.len()
+            )));
+        }
+
+        let mut output = [0u8; HASH_LENGTH];
+        HEXUPPER
+            .decode_mut(hex_string.as_bytes(), &mut output)
+            .expect("Hash decoding shouldn't fail");
+
+        Ok(Hash(output))
+    }
+}
+
 impl arse_merkle_tree::traits::Value for Hash {
     fn as_slice(&self) -> &[u8] {
         self.0.as_slice()