From a3fb40caac6111c6164b51600dcb151bf94d732a Mon Sep 17 00:00:00 2001 From: ardocrat Date: Sun, 26 Jul 2026 19:07:53 +0300 Subject: [PATCH] Wrong Slatepack address fix (#773) --- api/src/foreign.rs | 3 +- api/src/foreign_rpc.rs | 11 +++ api/src/owner.rs | 88 ++++++++++----------- api/src/owner_rpc.rs | 9 ++- controller/src/command.rs | 83 +++++++++++--------- controller/tests/config_api.rs | 2 +- controller/tests/slatepack.rs | 119 ++++++++++++++++++++++++++++- grin | 2 +- libwallet/src/api_impl/owner.rs | 15 +++- libwallet/src/slatepack/address.rs | 71 +++++++++++++++-- src/bin/grin-wallet.yml | 2 +- src/cmd/wallet_args.rs | 43 +++++++---- 12 files changed, 338 insertions(+), 110 deletions(-) diff --git a/api/src/foreign.rs b/api/src/foreign.rs index a7b0027..2d03c18 100644 --- a/api/src/foreign.rs +++ b/api/src/foreign.rs @@ -24,6 +24,7 @@ use crate::libwallet::{ use crate::try_slatepack_sync_workflow; use crate::util::secp::key::SecretKey; use crate::util::Mutex; +use libwallet::SlatepackAddress; use std::sync::Arc; @@ -343,7 +344,7 @@ where &self, slate: &Slate, dest_acct_name: Option<&str>, - r_addr: Option, + r_addr: Option, ) -> Result { let mut w_lock = self.wallet_inst.lock(); let tor_config = r_addr diff --git a/api/src/foreign_rpc.rs b/api/src/foreign_rpc.rs index 5cf0832..a0de987 100644 --- a/api/src/foreign_rpc.rs +++ b/api/src/foreign_rpc.rs @@ -22,6 +22,7 @@ use crate::libwallet::{ use crate::{Foreign, ForeignCheckMiddlewareFn}; use easy_jsonrpc_mw; use grin_wallet_config::initial_setup_wallet; +use libwallet::SlatepackAddress; use std::path::PathBuf; /// Public definition used to generate Foreign jsonrpc api. @@ -311,6 +312,16 @@ where ) -> Result { let version = in_slate.version(); let slate_from = Slate::from(in_slate); + let dest = match dest { + None => None, + Some(a) => match SlatepackAddress::try_from(a.as_str()) { + Ok(d) => Some(d), + Err(_) => { + error!("Error parsing Slatepack address: {}", a); + None + } + }, + }; let out_slate = Foreign::receive_tx( self, &slate_from, diff --git a/api/src/owner.rs b/api/src/owner.rs index 96b801c..9efc14f 100644 --- a/api/src/owner.rs +++ b/api/src/owner.rs @@ -703,6 +703,12 @@ where args: InitTxArgs, ) -> Result { let send_args = args.send_args.clone(); + let dest = match send_args.as_ref() { + Some(args) if !args.dest.is_empty() => { + Some(SlatepackAddress::try_from(args.dest.as_str())?) + } + _ => None, + }; let (slate, tor_config) = { let mut w_lock = self.wallet_inst.lock(); let tor_config = send_args @@ -721,10 +727,11 @@ where Error::TorConfig("Tor config was not loaded with send arguments".into()) })?; let can_send = tc.send_tor(sa.skip_tor); - if self.doctest_mode || !can_send { + if self.doctest_mode || !can_send || dest.is_none() { return Ok(slate); } - let res = try_slatepack_sync_workflow(&slate, &sa.dest, Some(tc), None, false); + let res = + try_slatepack_sync_workflow(&slate, &dest.unwrap(), Some(tc), None, false); match res { Ok(s) => { self.tx_lock_outputs(keychain_mask, &s)?; @@ -868,6 +875,12 @@ where args: InitTxArgs, ) -> Result { let send_args = args.send_args.clone(); + let dest = match send_args.as_ref() { + Some(args) if !args.dest.is_empty() => { + Some(SlatepackAddress::try_from(args.dest.as_str())?) + } + _ => None, + }; let (slate, tor_config) = { let mut w_lock = self.wallet_inst.lock(); let tor_config = send_args @@ -886,10 +899,11 @@ where Error::TorConfig("Tor config was not loaded with send arguments".into()) })?; let can_send = tc.send_tor(sa.skip_tor); - if self.doctest_mode || !can_send { + if self.doctest_mode || !can_send || dest.is_none() { return Ok(slate); } - let res = try_slatepack_sync_workflow(&slate, &sa.dest, Some(tc), None, true); + let dest = dest.unwrap(); + let res = try_slatepack_sync_workflow(&slate, &dest, Some(tc), None, true); match res { Ok(s) => { // Update slate state. @@ -903,7 +917,7 @@ where } } // Output slatepack message to file. - match output_slatepack_file(&self, keychain_mask, &s, &sa.dest) { + match output_slatepack_file(&self, keychain_mask, &s, Some(dest)) { Ok(_) => {} Err(e) => error!("Error on saving output slatepack message: {}", e), } @@ -2563,7 +2577,7 @@ where /// attempt to send slate synchronously with TOR pub fn try_slatepack_sync_workflow( slate: &Slate, - dest: &str, + dest: &SlatepackAddress, tor_config: Option, tor_sender: Option, send_to_finalize: bool, @@ -2585,42 +2599,32 @@ pub fn try_slatepack_sync_workflow( }; }; - // Try parsing Slatepack address. - match SlatepackAddress::try_from(dest) { - Ok(address) => { - let tor_addr = OnionV3Address::try_from(&address).map_err(|_| { - Error::SlatepackAddress(format!( - "Destination {} is not a valid Onion address.", - dest - )) - })?; - // Try sending to the destination via Tor. - let sender = match tor_sender { - None => { - if let Some(tc) = tor_config { - match TorSlateSender::new(&tor_addr.to_http_str(), tc) { - Ok(s) => s, - Err(e) => { - debug!("Send (Tor): Cannot create TOR Slate sender {:?}", e); - return Err(e); - } - } - } else { - return Err(Error::TorConfig("Tor config is not set".to_string())); + let tor_addr = OnionV3Address::try_from(dest).map_err(|_| { + Error::SlatepackAddress(format!( + "Destination {} is not a valid Onion address.", + dest + )) + })?; + let sender = match tor_sender { + None => { + if let Some(tc) = tor_config { + match TorSlateSender::new(&tor_addr.to_http_str(), tc) { + Ok(s) => s, + Err(e) => { + debug!("Send (Tor): Cannot create TOR Slate sender {:?}", e); + return Err(e); } } - Some(s) => s, - }; - warn!("Attempting to send transaction via Tor"); - match send_sync(sender, "Tor") { - Ok(_) => Ok(ret_slate), - Err(e) => Err(e), + } else { + return Err(Error::TorConfig("Tor config is not set".to_string())); } } - Err(e) => { - error!("Destination {} is not a valid Slatepack address.", dest); - Err(e) - } + Some(s) => s, + }; + warn!("Attempting to send transaction via Tor"); + match send_sync(sender, "Tor") { + Ok(_) => Ok(ret_slate), + Err(e) => Err(e), } } @@ -2699,19 +2703,15 @@ fn output_slatepack_file( api: &Owner, keychain_mask: Option<&SecretKey>, slate: &Slate, - dest: &str, + dest: Option, ) -> Result<(), Error> where L: WalletLCProvider<'static, C, K> + 'static, C: NodeClient + 'static, K: Keychain + 'static, { - let address = match SlatepackAddress::try_from(dest) { - Ok(a) => Some(a), - Err(_) => None, - }; // encrypt for recipient by default - let recipients = match address.clone() { + let recipients = match dest { Some(a) => vec![a], None => vec![], }; diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index f5d3a49..4c9c291 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -1721,7 +1721,7 @@ pub trait OwnerRpc { "params": { "token": "d202964900000000d302964900000000d402964900000000d502964900000000", "secret_indices": [0], - "message": "BEGINSLATEPACK. 8GQrdcwdLKJD28F 3a9siP7ZhZgAh7w BR2EiZHza5WMWmZ Cc8zBUemrrYRjhq j3VBwA8vYnvXXKU BDmQBN2yKgmR8mX UzvXHezfznA61d7 qFZYChhz94vd8Ew NEPLz7jmcVN2C3w wrfHbeiLubYozP2 uhLouFiYRrbe3fQ 4uhWGfT3sQYXScT dAeo29EaZJpfauh j8VL5jsxST2SPHq nzXFC2w9yYVjt7D ju7GSgHEp5aHz9R xstGbHjbsb4JQod kYLuELta1ohUwDD pvjhyJmsbLcsPei k5AQhZsJ8RJGBtY bou6cU7tZeFJvor 4LB9CBfFB3pmVWD vSLd5RPS75dcnHP nbXD8mSDZ8hJS2Q A9wgvppWzuWztJ2 dLUU8f9tLJgsRBw YZAs71HiVeg7. ENDSLATEPACK." + "message": "BEGINSLATEPACK. t9EcGgrKr1GFCQB SK2jPCxME6Hgpqx bntpQm3zKFycoPY nW4UeoL4KQ7ExNK At6EQsvpz6MjUs8 6WG8KHEbMfqufJQ ZJTw2gkcdJmJjiJ f29oGgYqqXDZox4 ujPSjrtoxCN4h3e i1sZ8dYsm3dPeXL 7VQLsYNjAefciqj ZJXPm4Pqd7VDdd4 okGBGBu3YJvYzT6 arAxeCEx66us31h AJLcDweFwyWBkW5 J1DLiYAjt5ftFTo CjpfW9KjiLq2LM5 jepXWEHJPSDAYVK 4macDZUhRbJiG6E hrQcPrJBVC716mb Hw5E1PFrE6on5wq oEmrS4j9vaB5nw8 Z9ZyXvPc2LN7tER yt6pSHZeY9EpYdY zv4bthzfRfF8ePT TMeMpV2gpgyRXQa CPD2TR. ENDSLATEPACK." }, "id": 1 } @@ -1865,9 +1865,9 @@ pub trait OwnerRpc { "proof": { "amount": "60000000000", "excess": "09eac5f5872fa5e08e0c29fd900f1b8f77ff3ad1d0d1c46aeb202cbf92363fe0af", - "recipient_address": "slatepack10qlk22rxjap2ny8qltc2tl996kenxr3hhwuu6hrzs6tdq08yaqgqnlumr7", + "recipient_address": "tgrin10qlk22rxjap2ny8qltc2tl996kenxr3hhwuu6hrzs6tdq08yaqgqq6t83r", "recipient_sig": "02868f2d2b983981f8f98043701687a8531ed2de564ea3df48e9e7e0229ccbe8359efe506896df2efbe3528e977252c50e4a41ca3cc9896e7c5a30bbb1d33604", - "sender_address": "slatepack1xtxavwfgs48ckf3gk8wwgcndmn0nt4tvkl8a7ltyejjcy2mc6nfskdvkdu", + "sender_address": "tgrin1xtxavwfgs48ckf3gk8wwgcndmn0nt4tvkl8a7ltyejjcy2mc6nfs9gm2lp", "sender_sig": "c511764f3f61ed3d1cbca9514df8bc6811fad5662b1cb0e0587b9c9e49db9f33183cce71af6cb24b507fabf525a2bc405c6e84e63a60334edff0b451ae5e6102" } }, @@ -2628,7 +2628,8 @@ pub fn run_doctest_owner( true => { let address = "783f6528669742a990e0faf0a5fca5d5b3330e37bbb9cd5c628696d03ce4e810"; let address = OnionV3Address::try_from(address).unwrap(); - Some(SlatepackAddress::try_from(address).unwrap()) + let sp = SlatepackAddress::try_from(address).unwrap(); + Some(sp) } false => None, }; diff --git a/controller/src/command.rs b/controller/src/command.rs index 397269c..dd6981a 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -33,7 +33,6 @@ use crate::{controller, display}; use qr_code::QrCode; use serde_json as json; -use std::convert::TryFrom; use std::fs::File; use std::io::{Read, Write}; use std::sync::atomic::Ordering; @@ -315,12 +314,12 @@ pub struct SendArgs { pub selection_strategy: String, pub estimate_selection_strategies: bool, pub late_lock: bool, - pub dest: String, + pub dest: Option, pub change_outputs: usize, pub fluff: bool, pub max_outputs: usize, pub target_slate_version: Option, - pub payment_proof_address: Option, + pub payment_proof_address: Option, pub ttl_blocks: Option, pub skip_tor: Option, pub outfile: Option, @@ -341,6 +340,15 @@ where C: NodeClient + 'static, K: keychain::Keychain + 'static, { + let dest = if args.estimate_selection_strategies { + None + } else { + args.dest + .as_deref() + .map(SlatepackAddress::try_from) + .transpose()? + }; + let mut slate = Slate::blank(2, false); let mut amount = args.amount; if args.use_max_amount { @@ -369,6 +377,11 @@ where .collect::, grin_wallet_libwallet::Error>>()?; display::estimate(amount, strategies, dark_scheme); } else { + let payment_proof_recipient_address = args + .payment_proof_address + .as_deref() + .map(SlatepackAddress::try_from) + .transpose()?; let init_args = InitTxArgs { src_acct_name: None, amount, @@ -378,7 +391,7 @@ where num_change_outputs: args.change_outputs as u32, selection_strategy_is_use_all: args.selection_strategy == "all", target_slate_version: args.target_slate_version, - payment_proof_recipient_address: args.payment_proof_address.clone(), + payment_proof_recipient_address, ttl_blocks: args.ttl_blocks, send_args: None, late_lock: Some(args.late_lock), @@ -390,7 +403,9 @@ where info!( "Tx created: {} grin to {} (strategy '{}')", core::amount_to_hr_string(amount, false), - args.dest, + dest.as_ref() + .map(ToString::to_string) + .unwrap_or_else(|| "no destination".to_string()), args.selection_strategy, ); s @@ -415,7 +430,7 @@ where owner_api, keychain_mask, &slate, - args.dest.as_str(), + dest.clone(), args.outfile, true, false, @@ -424,11 +439,12 @@ where }; let can_send = tor_config.send_tor(args.skip_tor); - if test_mode || !can_send { + if test_mode || !can_send || dest.is_none() { return output_sp(); } - let res = try_slatepack_sync_workflow(&slate, &args.dest, Some(tor_config), None, false); + let dest = dest.as_ref().unwrap(); + let res = try_slatepack_sync_workflow(&slate, dest, Some(tor_config), None, false); match res { Ok(s) => { @@ -457,7 +473,7 @@ pub fn output_slatepack( owner_api: &mut Owner, keychain_mask: Option<&SecretKey>, slate: &Slate, - dest: &str, + dest: Option, out_file_override: Option, lock: bool, finalizing: bool, @@ -469,12 +485,8 @@ where K: keychain::Keychain + 'static, { // Output the slatepack file to stdout and to a file - let address = match SlatepackAddress::try_from(dest) { - Ok(a) => Some(a), - Err(_) => None, - }; // encrypt for recipient by default - let recipients = match address.clone() { + let recipients = match dest.clone() { Some(a) => vec![a], None => vec![], }; @@ -520,7 +532,7 @@ where println!(); } } - if address.is_some() { + if dest.is_some() { println!("The slatepack data is encrypted for the recipient only"); } else { println!("The slatepack data is NOT encrypted"); @@ -638,17 +650,12 @@ where }, )?; - let dest = match ret_address { - Some(a) => String::try_from(&a)?, - None => String::from(""), - }; - let output_sp = || -> Result<(), Error> { Ok(output_slatepack( owner_api, keychain_mask, &slate, - &dest, + ret_address.clone(), args.outfile, false, false, @@ -657,11 +664,12 @@ where }; let can_send = tor_config.send_tor(args.skip_tor); - if test_mode || !can_send { + if test_mode || !can_send || ret_address.is_none() { return output_sp(); } - let res = try_slatepack_sync_workflow(&slate, &dest, Some(tor_config), None, true); + let dest = ret_address.as_ref().unwrap(); + let res = try_slatepack_sync_workflow(&slate, dest, Some(tor_config), None, true); match res { Ok(s) => { @@ -829,7 +837,7 @@ where owner_api, keychain_mask, &slate, - "", + None, args.outfile, false, true, @@ -842,7 +850,7 @@ where /// Issue Invoice Args pub struct IssueInvoiceArgs { /// Slatepack address - pub dest: String, + pub dest: Option, /// issue invoice tx args pub issue_args: IssueInvoiceTxArgs, /// output file override @@ -861,6 +869,11 @@ where C: NodeClient + 'static, K: keychain::Keychain + 'static, { + let dest = args + .dest + .as_deref() + .map(SlatepackAddress::try_from) + .transpose()?; let issue_args = args.issue_args.clone(); let slate = owner_api.issue_invoice_tx(keychain_mask, issue_args)?; @@ -869,7 +882,7 @@ where owner_api, keychain_mask, &slate, - args.dest.as_str(), + dest, args.outfile, false, false, @@ -882,7 +895,7 @@ where pub struct ProcessInvoiceArgs { pub minimum_confirmations: u64, pub selection_strategy: String, - pub ret_address: Option, + pub ret_address: Option, pub max_outputs: usize, pub slate: Slate, pub estimate_selection_strategies: bool, @@ -907,11 +920,12 @@ where C: NodeClient + 'static, K: keychain::Keychain + 'static, { + let ret_address = args + .ret_address + .as_deref() + .map(SlatepackAddress::try_from) + .transpose()?; let mut slate = args.slate.clone(); - let dest = match args.ret_address.clone() { - Some(a) => String::try_from(&a)?, - None => String::from(""), - }; if args.estimate_selection_strategies { let strategies = vec!["smallest", "all"] @@ -970,7 +984,7 @@ where owner_api, keychain_mask, &slate, - &dest, + ret_address.clone(), args.outfile, true, false, @@ -979,11 +993,12 @@ where }; let can_send = tor_config.send_tor(args.skip_tor); - if test_mode || !can_send { + if test_mode || !can_send || ret_address.is_none() { return output_sp(); } - let res = try_slatepack_sync_workflow(&slate, &dest, Some(tor_config), None, true); + let dest = ret_address.as_ref().unwrap(); + let res = try_slatepack_sync_workflow(&slate, dest, Some(tor_config), None, true); match res { Ok(s) => { diff --git a/controller/tests/config_api.rs b/controller/tests/config_api.rs index fd37e10..5936a7f 100644 --- a/controller/tests/config_api.rs +++ b/controller/tests/config_api.rs @@ -129,7 +129,7 @@ fn send_preflight() { let args = InitTxArgs { amount: 1, send_args: Some(InitTxSendArgs { - dest: "unused".into(), + dest: "tgrin1xtxavwfgs48ckf3gk8wwgcndmn0nt4tvkl8a7ltyejjcy2mc6nfs9gm2lp".into(), post_tx: false, fluff: false, skip_tor: Some(true), diff --git a/controller/tests/slatepack.rs b/controller/tests/slatepack.rs index cf2a7e1..9bdd740 100644 --- a/controller/tests/slatepack.rs +++ b/controller/tests/slatepack.rs @@ -18,6 +18,7 @@ extern crate grin_wallet_controller as wallet; extern crate grin_wallet_impls as impls; use grin_core as core; +use grin_wallet_config::GlobalWalletConfig; use grin_wallet_libwallet as libwallet; use std::path::PathBuf; @@ -28,8 +29,8 @@ use std::thread; use std::time::Duration; use grin_wallet_libwallet::{ - InitTxArgs, IssueInvoiceTxArgs, Slate, Slatepack, SlatepackAddress, Slatepacker, - SlatepackerArgs, + InitTxArgs, InitTxSendArgs, IssueInvoiceTxArgs, Slate, Slatepack, SlatepackAddress, + Slatepacker, SlatepackerArgs, }; use ed25519_dalek::SigningKey as edDalekSecretKey; @@ -601,6 +602,110 @@ fn slatepack_api_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { Ok(()) } +/// Do not create transaction for invalid or wrong network Slatepack address. +fn slatepack_address_validation(test_dir: &'static str) -> Result<(), libwallet::Error> { + let mut wallet_proxy = create_wallet_proxy(test_dir); + let chain = wallet_proxy.chain.clone(); + let stopper = wallet_proxy.running.clone(); + let config_path = PathBuf::from(test_dir).join("grin-wallet.toml"); + GlobalWalletConfig::for_chain(&core::global::ChainTypes::AutomatedTesting, &config_path) + .write_to_file(config_path.to_str().unwrap(), false, None, None) + .unwrap(); + + create_wallet_and_add!( + client1, + wallet1, + mask1_i, + test_dir, + "wallet1", + None, + &mut wallet_proxy, + false + ); + let mask1 = (&mask1_i).as_ref(); + + create_wallet_and_add!( + client2, + wallet2, + mask2_i, + test_dir, + "wallet2", + None, + &mut wallet_proxy, + false + ); + let mask2 = (&mask2_i).as_ref(); + + let proxy_thread = thread::spawn(move || { + if let Err(e) = wallet_proxy.run() { + error!("Wallet Proxy error: {}", e); + } + }); + + let reward = core::consensus::REWARD; + let _ = test_framework::award_blocks_to_wallet(&chain, wallet1.clone(), mask1, 6, false); + let mut slate = Slate::blank(2, true); + + wallet::controller::owner_single_use(wallet2.clone(), mask2, config_path.clone(), |api, m| { + slate = api.issue_invoice_tx( + m, + IssueInvoiceTxArgs { + amount: reward, + ..Default::default() + }, + )?; + Ok(()) + })?; + + wallet::controller::owner_single_use(wallet1.clone(), mask1, config_path, |api, m| { + let args = InitTxArgs { + src_acct_name: Some("mining".to_owned()), + amount: reward, + minimum_confirmations: 2, + max_outputs: 500, + num_change_outputs: 1, + selection_strategy_is_use_all: true, + ..Default::default() + }; + + let mut wrong_net_args = args.clone(); + wrong_net_args.send_args = Some(InitTxSendArgs { + dest: "grin1dvge9z4uqgqlpspmljrd7smh3grrw9xu2r9lkz3u67s3emj3ud2sd5gk9p".to_string(), + post_tx: false, + fluff: false, + skip_tor: Some(true), + }); + assert!(api.init_send_tx(m, wrong_net_args.clone()).is_err()); + assert!(api.process_invoice_tx(m, &slate, wrong_net_args).is_err()); + + let mut invalid_args = args.clone(); + invalid_args.send_args = Some(InitTxSendArgs { + dest: "tgrinaddr10qlk22rxjap2ny8qltc2tl996kenxr3hhwuu6hrzs6tdq08yaqgqnlumr7" + .to_string(), + post_tx: false, + fluff: false, + skip_tor: Some(true), + }); + assert!(api.init_send_tx(m, invalid_args.clone()).is_err()); + assert!(api.process_invoice_tx(m, &slate, invalid_args).is_err()); + + let mut valid_args = args; + valid_args.send_args = Some(InitTxSendArgs { + dest: "tgrin1xtxavwfgs48ckf3gk8wwgcndmn0nt4tvkl8a7ltyejjcy2mc6nfs9gm2lp".to_string(), + post_tx: false, + fluff: false, + skip_tor: Some(true), + }); + api.process_invoice_tx(m, &slate, valid_args.clone())?; + api.init_send_tx(m, valid_args)?; + Ok(()) + })?; + + stopper.store(false, Ordering::Relaxed); + proxy_thread.join().expect("wallet proxy thread panicked"); + Ok(()) +} + #[test] fn slatepack_exchange_json() { let test_dir = "test_output/slatepack_exchange_json"; @@ -677,3 +782,13 @@ fn slatepack_api() { } clean_output_dir(test_dir); } + +#[test] +fn slatepack_address() { + let test_dir = "test_output/slatepack_address"; + setup(test_dir); + if let Err(e) = slatepack_address_validation(test_dir) { + panic!("Libwallet Error: {}", e); + } + clean_output_dir(test_dir); +} diff --git a/grin b/grin index bad44b4..44bc3dc 160000 --- a/grin +++ b/grin @@ -1 +1 @@ -Subproject commit bad44b4e9bd7e37417d80cc469bcba316249bd93 +Subproject commit 44bc3dc6918dc5600feaebb0e452dc4b1a9b2e42 diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index 504df14..a7a6691 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -470,6 +470,19 @@ where C: NodeClient, K: Keychain, { + let payment_proof_address = if let Some(a) = &args.payment_proof_recipient_address { + if a.valid_network() { + Some(a) + } else { + return Err(Error::PaymentProofRetrieval(format!( + "Wrong network for address {}", + a + ))); + } + } else { + None + }; + let parent_key_id = match &args.src_acct_name { Some(d) => { let pm = w.get_acct_path(d.clone())?; @@ -539,7 +552,7 @@ where // probably want to allow sender to specify which one let deriv_path = 0u32; - if let Some(a) = args.payment_proof_recipient_address { + if let Some(a) = payment_proof_address { let k = w.keychain(keychain_mask)?; let sec_addr_key = address::address_from_derivation_path(&k, &parent_key_id, deriv_path)?; diff --git a/libwallet/src/slatepack/address.rs b/libwallet/src/slatepack/address.rs index 1e88541..9368e38 100644 --- a/libwallet/src/slatepack/address.rs +++ b/libwallet/src/slatepack/address.rs @@ -41,15 +41,18 @@ pub struct SlatepackAddress { pub pub_key: edDalekPublicKey, } +fn slatepack_hrp() -> &'static str { + match global::get_chain_type() { + global::ChainTypes::Mainnet => "grin", + _ => "tgrin", + } +} + impl SlatepackAddress { /// new with default hrp pub fn new(pub_key: &edDalekPublicKey) -> Self { - let hrp = match global::get_chain_type() { - global::ChainTypes::Mainnet => "grin", - _ => "tgrin", - }; Self { - hrp: String::from(hrp), + hrp: String::from(slatepack_hrp()), pub_key: pub_key.clone(), } } @@ -75,11 +78,19 @@ impl SlatepackAddress { let x_key = xDalekPublicKey::try_from(self)?; Ok(bech32::encode("age", x_key.as_bytes().to_base32())?.to_string()) } + + /// Check if address network is valid. + pub fn valid_network(&self) -> bool { + valid_network(&self.hrp) + } } impl Display for SlatepackAddress { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str(&String::try_from(self).unwrap()) + match String::try_from(self) { + Ok(encoded) => formatter.write_str(&encoded), + Err(_) => formatter.write_str(""), + } } } @@ -87,6 +98,12 @@ impl TryFrom<&str> for SlatepackAddress { type Error = Error; fn try_from(encoded: &str) -> Result { let (hrp, data) = bech32::decode(&encoded)?; + if !valid_network(&hrp) { + return Err(Error::SlatepackAddress(format!( + "wrong address prefix for chain {:?}", + global::get_chain_type() + ))); + } let bytes = Vec::::from_base32(&data)?; let b = <&[u8; 32]>::try_from(bytes.as_slice()) .map_err(|_| Error::SlatepackAddress("Wrong encoded data".to_string()))?; @@ -242,10 +259,52 @@ impl Readable for SlatepackAddress { } } +/// Check if encoded HRP is for valid network. +fn valid_network(hrp: &str) -> bool { + slatepack_hrp() == hrp +} + #[test] fn slatepack_address() -> Result<(), Error> { use rand::{thread_rng, Rng}; global::set_local_chain_type(global::ChainTypes::AutomatedTesting); + + // Check validation. + let bytes: [u8; 32] = thread_rng().gen(); + let pub_key = edDalekPublicKey::from(&edDalekSecretKey::from_bytes(&bytes)); + let wrong_net_addr = SlatepackAddress { + hrp: "grin".to_string(), + pub_key, + }; + assert!(!wrong_net_addr.valid_network()); + let right_addr = SlatepackAddress { + hrp: "tgrin".to_string(), + pub_key, + }; + assert!(right_addr.valid_network()); + let invalid_addr = SlatepackAddress { + hrp: "invalid hrp".to_string(), + pub_key, + }; + assert!(!invalid_addr.valid_network()); + assert_eq!(invalid_addr.to_string(), ""); + + let valid_addr = "tgrin1xtxavwfgs48ckf3gk8wwgcndmn0nt4tvkl8a7ltyejjcy2mc6nfs9gm2lp"; + let parsed_valid_addr = SlatepackAddress::try_from(valid_addr); + assert!(parsed_valid_addr.is_ok()); + + let valid_addr2 = "TGRIN1XTXAVWFGS48CKF3GK8WWGCNDMN0NT4TVKL8A7LTYEJJCY2MC6NFS9GM2LP"; + let parsed_valid_addr2 = SlatepackAddress::try_from(valid_addr2); + assert!(parsed_valid_addr2.is_ok()); + + let invalid_addr = "slatepack10qlk22rxjap2ny8qltc2tl996kenxr3hhwuu6hrzs6tdq08yaqgqnlumr7"; + let parsed_invalid_addr = SlatepackAddress::try_from(invalid_addr); + assert!(parsed_invalid_addr.is_err()); + + let wrong_net_addr = "grin1dvge9z4uqgqlpspmljrd7smh3grrw9xu2r9lkz3u67s3emj3ud2sd5gk9p"; + let parsed_wrong_net_addr = SlatepackAddress::try_from(wrong_net_addr); + assert!(parsed_wrong_net_addr.is_err()); + let sec_key_bytes: [u8; 32] = thread_rng().gen(); let ed_sec_key = edDalekSecretKey::from_bytes(&sec_key_bytes); diff --git a/src/bin/grin-wallet.yml b/src/bin/grin-wallet.yml index 39a8962..93a4342 100644 --- a/src/bin/grin-wallet.yml +++ b/src/bin/grin-wallet.yml @@ -132,7 +132,7 @@ subcommands: default_value: "1" takes_value: true - dest: - help: Intended recipient's Slatepack Address (or http listener address (DEPRECATED)) + help: Intended recipient's Slatepack Address short: d long: dest takes_value: true diff --git a/src/cmd/wallet_args.rs b/src/cmd/wallet_args.rs index 2bce63f..3446eaf 100644 --- a/src/cmd/wallet_args.rs +++ b/src/cmd/wallet_args.rs @@ -466,7 +466,11 @@ pub fn parse_send_args(args: &ArgMatches) -> Result Result match SlatepackAddress::try_from(dest) { - Ok(a) => Some(a), - Err(_) => { - if !estimate_selection_strategies { - println!("No recipient Slatepack address or provided address invalid. No payment proof will be requested."); - } - None - } - }, - true => None, + if let Some(a) = dest.clone() { + match args.is_present("no_payment_proof") { + false => Some(a), + true => None, + } + } else { + if !estimate_selection_strategies { + println!("No recipient Slatepack address or provided address invalid. No payment proof will be requested."); + } + None } }; @@ -530,7 +533,7 @@ pub fn parse_send_args(args: &ArgMatches) -> Result