From a1d683294b70f74e276d6607b6926ddd35308eb2 Mon Sep 17 00:00:00 2001 From: wiesche Date: Tue, 15 Sep 2026 20:28:20 +0200 Subject: [PATCH] fix early proof export and slatepack account errors --- controller/src/command.rs | 120 +++++++++++++++--- controller/src/display.rs | 36 ++++-- controller/tests/contract_early_proofs_srs.rs | 43 +++++++ controller/tests/contract_rsr.rs | 2 + controller/tests/contract_srs.rs | 2 + controller/tests/slatepack.rs | 11 ++ doc/contracts.md | 2 + libwallet/src/api_impl/owner.rs | 48 ++++++- libwallet/src/contract/utils.rs | 2 + tests/cmd_line_contract.rs | 56 ++++++++ 10 files changed, 290 insertions(+), 32 deletions(-) diff --git a/controller/src/command.rs b/controller/src/command.rs index 3194d4b..054e792 100644 --- a/controller/src/command.rs +++ b/controller/src/command.rs @@ -27,8 +27,8 @@ use crate::libwallet::contract::types::{ }; use crate::libwallet::contract::{can_finalize, initial_net_change}; use crate::libwallet::{ - self, InitTxArgs, IssueInvoiceTxArgs, NodeClient, PaymentProof, Slate, SlateState, - SlatepackAddress, Slatepacker, SlatepackerArgs, WalletLCProvider, + self, EarlyPaymentProof, InitTxArgs, IssueInvoiceTxArgs, NodeClient, PaymentProof, Slate, + SlateState, SlatepackAddress, Slatepacker, SlatepackerArgs, WalletLCProvider, }; use crate::util::secp::key::SecretKey; use crate::util::{Mutex, ZeroingString}; @@ -774,7 +774,22 @@ where let was_encrypted = slatepack.is_encrypted(); if was_encrypted { let dec_key = owner_api.get_slatepack_secret_key(keychain_mask, 0)?; - slatepack.try_decrypt_payload(Some(&dec_key))?; + if let Err(e) = slatepack.try_decrypt_payload(Some(&dec_key)) { + // tell the user which account can decrypt it + let account = libwallet::api_impl::owner::slatepack_account( + owner_api.wallet_inst.clone(), + keychain_mask, + &slatepack, + )?; + return Err(match account { + Some(label) => libwallet::Error::SlatepackDecryption(format!( + "it is encrypted for account '{}' of this wallet, select it with --account {}", + label, label + )) + .into(), + None => e.into(), + }); + } } let slate = packer.get_slate(&slatepack)?; Ok((slate, slatepack.sender, was_encrypted)) @@ -1554,6 +1569,13 @@ pub struct ProofExportArgs { pub tx_slate_id: Option, } +/// Exported early proof, with the receiver address needed to verify it +#[derive(Serialize, Deserialize)] +struct EarlyProofFile { + recipient_address: SlatepackAddress, + proof: EarlyPaymentProof, +} + pub fn proof_export( owner_api: &mut Owner, keychain_mask: Option<&SecretKey>, @@ -1564,14 +1586,34 @@ where C: NodeClient + 'static, K: keychain::Keychain + 'static, { - let result = owner_api.retrieve_payment_proof(keychain_mask, true, args.id, args.tx_slate_id); + // early proofs have no sender signature + let (_, txs) = owner_api.retrieve_txs(keychain_mask, false, args.id, args.tx_slate_id, None)?; + let early_recipient = txs + .first() + .and_then(|tx| tx.payment_proof.as_ref()) + .filter(|p| p.proof_type.is_some()) + .map(|p| SlatepackAddress::new(&p.receiver_address)); + let result = match early_recipient { + Some(recipient_address) => owner_api + .retrieve_payment_proof_early(keychain_mask, true, args.id, args.tx_slate_id) + .map(|proof| { + json::to_string_pretty(&EarlyProofFile { + recipient_address, + proof, + }) + .unwrap() + }), + None => owner_api + .retrieve_payment_proof(keychain_mask, true, args.id, args.tx_slate_id) + .map(|p| json::to_string_pretty(&p).unwrap()), + }; match result { Ok(p) => { // actually export proof let mut proof_file = File::create(args.output_file.clone()) .map_err(|e| Error::GenericError(format!("{}", e)))?; proof_file - .write_all(json::to_string_pretty(&p).unwrap().as_bytes()) + .write_all(p.as_bytes()) .map_err(|e| Error::GenericError(format!("{}", e)))?; proof_file .sync_all() @@ -1616,6 +1658,13 @@ where proof_f .read_to_string(&mut proof) .map_err(|e| Error::GenericError(format!("{}", e)))?; + // early proof file + let is_early = json::from_str::(&proof) + .map(|v| v.get("proof").is_some()) + .unwrap_or(false); + if is_early { + return proof_verify_early(owner_api, keychain_mask, &proof); + } // read let proof: PaymentProof = match json::from_str(&proof) { Ok(p) => p, @@ -1628,18 +1677,7 @@ where let result = owner_api.verify_payment_proof(keychain_mask, &proof); match result { Ok((iam_sender, iam_recipient)) => { - println!("Payment proof's signatures are valid."); - if iam_sender { - println!("The proof's sender address belongs to this wallet."); - } - if iam_recipient { - println!("The proof's recipient address belongs to this wallet."); - } - if !iam_recipient && !iam_sender { - println!( - "Neither the proof's sender nor recipient address belongs to this wallet." - ); - } + print_proof_verified(iam_sender, iam_recipient); Ok(()) } Err(e) => { @@ -1649,6 +1687,54 @@ where } } +fn print_proof_verified(iam_sender: bool, iam_recipient: bool) { + println!("Payment proof's signatures are valid."); + if iam_sender { + println!("The proof's sender address belongs to this wallet."); + } + if iam_recipient { + println!("The proof's recipient address belongs to this wallet."); + } + if !iam_recipient && !iam_sender { + println!("Neither the proof's sender nor recipient address belongs to this wallet."); + } +} + +fn proof_verify_early( + owner_api: &mut Owner, + keychain_mask: Option<&SecretKey>, + proof: &str, +) -> Result<(), Error> +where + L: WalletLCProvider<'static, C, K> + 'static, + C: NodeClient + 'static, + K: keychain::Keychain + 'static, +{ + let file: EarlyProofFile = json::from_str(proof).map_err(|e| { + error!("Unable to parse payment proof file: {}", e); + Error::from(libwallet::Error::PaymentProofParsing(format!("{}", e))) + })?; + let result = { + let mut w_lock = owner_api.wallet_inst.lock(); + let w = w_lock.lc_provider()?.wallet_inst()?; + libwallet::api_impl::foreign::verify_payment_proof_early( + w, + &file.recipient_address.pub_key, + &file.proof, + ) + }; + if let Err(e) = result { + error!("Proof not valid: {}", e); + return Err(Error::from(e)); + } + let my_address = owner_api.get_slatepack_address(keychain_mask, 0)?.pub_key; + print_proof_verified( + file.proof.sender_address == my_address, + file.recipient_address.pub_key == my_address, + ); + Ok(()) +} + /// Create new contract command arguments #[derive(Clone)] pub struct ContractNewArgs { diff --git a/controller/src/display.rs b/controller/src/display.rs index deca5a3..427078a 100644 --- a/controller/src/display.rs +++ b/controller/src/display.rs @@ -598,14 +598,7 @@ pub fn payment_proof(tx: &TxLogEntry) -> Result<(), Error> { Some(f) => f.fee(), // apply fee mask past HF4 None => 0, }; - let amount = if tx.amount_credited >= tx.amount_debited { - core::amount_to_hr_string(tx.amount_credited - tx.amount_debited, true) - } else { - format!( - "{}", - core::amount_to_hr_string(tx.amount_debited - tx.amount_credited - fee, true) - ) - }; + let amount = core::amount_to_hr_string(proof_amount(tx, pp.proof_type.is_some(), fee), true); let sender_signature = match pp.sender_signature { Some(s) => { @@ -647,6 +640,15 @@ pub fn payment_proof(tx: &TxLogEntry) -> Result<(), Error> { Ok(()) } +/// Contract tx logs store the net amount, so early proofs don't subtract the fee +fn proof_amount(tx: &TxLogEntry, early_proof: bool, fee: u64) -> u64 { + if early_proof || tx.amount_credited >= tx.amount_debited { + tx.amount_credited.abs_diff(tx.amount_debited) + } else { + tx.amount_debited - tx.amount_credited - fee + } +} + /// Display a summary of a contract slate pub fn contract_view(slate: &Slate, view: &ContractView, was_encrypted: bool) { println!("\n____ Contract ____\n"); @@ -766,11 +768,12 @@ fn format_net_change(change: i64) -> String { #[cfg(test)] mod tests { - use super::{contract_view_table, format_net_change}; + use super::{contract_view_table, format_net_change, proof_amount}; use crate::core::core::{FeeFields, Input, Inputs, Output, OutputFeatures, Transaction}; use crate::core::global; + use crate::keychain::Identifier; use crate::libwallet::contract::types::ContractView; - use crate::libwallet::Slate; + use crate::libwallet::{Slate, TxLogEntry, TxLogEntryType}; use crate::util::secp::pedersen::{Commitment, RangeProof}; fn table_value(table: &prettytable::Table, label: &str) -> String { @@ -794,6 +797,19 @@ mod tests { assert_eq!(format_net_change(0), "+0.000000000"); } + #[test] + fn proof_amount_legacy_and_early() { + let mut tx = TxLogEntry::new(Identifier::zero(), TxLogEntryType::TxSent, 0); + // legacy send + tx.amount_debited = 1_500_000_000; + tx.amount_credited = 477_000_000; + assert_eq!(proof_amount(&tx, false, 23_000_000), 1_000_000_000); + // contract send + tx.amount_debited = 200_000_000; + tx.amount_credited = 0; + assert_eq!(proof_amount(&tx, true, 11_750_000), 200_000_000); + } + #[test] fn contract_view_rows() { global::set_local_chain_type(global::ChainTypes::AutomatedTesting); diff --git a/controller/tests/contract_early_proofs_srs.rs b/controller/tests/contract_early_proofs_srs.rs index ec8929c..af0bb26 100644 --- a/controller/tests/contract_early_proofs_srs.rs +++ b/controller/tests/contract_early_proofs_srs.rs @@ -240,6 +240,49 @@ fn contract_early_proofs_srs_test_impl(test_dir: &'static str) -> Result<(), lib }, )?; + // export and verify through the CLI + let proof_file = format!("{}/early_proof.json", test_dir); + let tampered_file = format!("{}/early_proof_tampered.json", test_dir); + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + wallet::command::proof_export( + api, + m, + wallet::command::ProofExportArgs { + output_file: proof_file.clone(), + id: None, + tx_slate_id: Some(slate.id), + }, + ) + .map_err(|e| libwallet::Error::GenericError(e.to_string()))?; + wallet::command::proof_verify( + api, + m, + wallet::command::ProofVerifyArgs { + input_file: proof_file.clone(), + }, + ) + .map_err(|e| libwallet::Error::GenericError(e.to_string()))?; + // tweak the amount and it shouldn't verify + let mut tampered: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(&proof_file).unwrap()).unwrap(); + tampered["proof"]["amount"] = serde_json::Value::from("400000"); + std::fs::write(&tampered_file, tampered.to_string()).unwrap(); + let retval = wallet::command::proof_verify( + api, + m, + wallet::command::ProofVerifyArgs { + input_file: tampered_file.clone(), + }, + ); + assert!(retval.is_err()); + Ok(()) + }, + )?; + // let logging finish stopper.store(false, Ordering::Relaxed); thread::sleep(Duration::from_millis(200)); diff --git a/controller/tests/contract_rsr.rs b/controller/tests/contract_rsr.rs index 3bab5e5..e18edcb 100644 --- a/controller/tests/contract_rsr.rs +++ b/controller/tests/contract_rsr.rs @@ -224,6 +224,7 @@ fn contract_rsr_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(txs.len(), 5); // 4 mined and 1 received let tx_log = common::tx_log_for_slate(api, m, &slate)?; assert_eq!(tx_log.tx_type, TxLogEntryType::TxReceived); + assert_eq!(tx_log.tx_slate_state, Some(SlateState::Invoice3)); assert_eq!(tx_log.amount_credited, 5_000_000_000); assert_eq!(tx_log.amount_debited, 0); assert_eq!(tx_log.num_inputs, 1); @@ -251,6 +252,7 @@ fn contract_rsr_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(txs.len() as u64, bh - 4 + 1); // send_wallet didn't mine 4 blocks and made 1 tx let tx_log = common::tx_log_for_slate(api, m, &slate)?; assert_eq!(tx_log.tx_type, TxLogEntryType::TxSent); + assert_eq!(tx_log.tx_slate_state, Some(SlateState::Invoice2)); assert_eq!(tx_log.amount_credited, 0); assert_eq!(tx_log.amount_debited, 5_000_000_000); assert_eq!(tx_log.num_inputs, 1); diff --git a/controller/tests/contract_srs.rs b/controller/tests/contract_srs.rs index 96647e9..dd4215e 100644 --- a/controller/tests/contract_srs.rs +++ b/controller/tests/contract_srs.rs @@ -631,6 +631,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(txs.len(), 5); // 4 mined and 1 received let tx_log = common::tx_log_for_slate(api, m, &slate)?; assert_eq!(tx_log.tx_type, TxLogEntryType::TxReceived); + assert_eq!(tx_log.tx_slate_state, Some(SlateState::Standard2)); assert_eq!(tx_log.amount_credited, 5_000_000_000); assert_eq!(tx_log.amount_debited, 0); assert_eq!(tx_log.num_inputs, 1); @@ -657,6 +658,7 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(txs.len() as u64, bh - 4 + 1); // send wallet didn't mine 4 blocks and made 1 tx let tx_log = common::tx_log_for_slate(api, m, &slate)?; assert_eq!(tx_log.tx_type, TxLogEntryType::TxSent); + assert_eq!(tx_log.tx_slate_state, Some(SlateState::Standard3)); assert_eq!(tx_log.amount_credited, 0); assert_eq!(tx_log.amount_debited, 5_000_000_000); assert_eq!(tx_log.num_inputs, 1); diff --git a/controller/tests/slatepack.rs b/controller/tests/slatepack.rs index b1a3e36..9322d95 100644 --- a/controller/tests/slatepack.rs +++ b/controller/tests/slatepack.rs @@ -609,6 +609,17 @@ fn slatepack_api_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { println!("{}", slatepack_raw); let decoded_slate = api.slate_from_slatepack_message(m, slatepack, vec![0])?; println!("{}", decoded_slate); + // wrong active account, the error names the right one + let own_address = api.get_slatepack_address(m, 0)?; + let own_slatepack = + api.create_slatepack_message(m, &slate, Some(0), vec![own_address])?; + api.create_account_path(m, "second")?; + api.set_active_account(m, "second")?; + let err = api + .slate_from_slatepack_message(m, own_slatepack, vec![0]) + .unwrap_err(); + assert!(err.to_string().contains("account 'default'"), "{}", err); + api.set_active_account(m, "default")?; Ok(()) }, )?; diff --git a/doc/contracts.md b/doc/contracts.md index f036f97..706b333 100644 --- a/doc/contracts.md +++ b/doc/contracts.md @@ -52,6 +52,7 @@ does not, revoke the contract and create a new one. Each side's account is fixed when it first joins the contract. Changing the active account later does not affect setup or signing; they use the account stored in the context. +Encrypted Slatepacks still need their account selected with `--account`; the error names it. ## View and revoke @@ -80,6 +81,7 @@ account selected with `--account`. the [known multi-party attack](https://forum.grin.mw/t/grin-wallet-contract-prototype/9745/18) * Early payment proofs are available for contracts through the API and CLI. Use `--proof-type` and optionally `--memo` (up to 1024 bytes of UTF-8 text). The proof binds the memo's Blake2b hash +* `export_proof` and `verify_proof` also handle early proofs * Invoice proofs work with SRS and RSR. Sender-nonce proofs only work with RSR. Both need experimental Slate V5; see [Early Payment Proofs](https://github.com/mimblewimble/grin-rfcs/pull/70) diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index c38b2c1..c2fd727 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -142,6 +142,31 @@ where Ok(d_skey) } +/// Label of another account that can decrypt the slatepack, if any +pub fn slatepack_account<'a, L, C, K>( + wallet_inst: Arc>>>, + keychain_mask: Option<&SecretKey>, + slatepack: &Slatepack, +) -> Result, Error> +where + L: WalletLCProvider<'a, C, K>, + C: NodeClient + 'a, + K: Keychain + 'a, +{ + wallet_lock!(wallet_inst, w); + let active = w.parent_key_id(); + let k = w.keychain(keychain_mask)?; + for acct in w.acct_path_iter()?.filter(|acct| acct.path != active) { + let sec_addr_key = address::address_from_derivation_path(&k, &acct.path, 0)?; + let dec_key = DalekSecretKey::from_bytes(&sec_addr_key.0); + let mut candidate = slatepack.clone(); + if candidate.try_decrypt_payload(Some(&dec_key)).is_ok() { + return Ok(Some(acct.label)); + } + } + Ok(None) +} + /// Create a slatepack message from the given slate pub fn create_slatepack_message<'a, L, C, K>( wallet_inst: Arc>>>, @@ -217,11 +242,24 @@ where }; return packer.get_slate(&slatepack); } - Err(Error::SlatepackDecryption( - "Could not decrypt slatepack with any provided index on the address derivation path" - .to_owned(), - ) - .into()) + // maybe it's for another account + let packer = Slatepacker::new(SlatepackerArgs { + sender: None, + recipients: vec![], + dec_key: None, + }); + let encrypted = packer.deser_slatepack(slatepack.as_bytes(), false)?; + let msg = match slatepack_account(wallet_inst, keychain_mask, &encrypted)? { + Some(label) => format!( + "it is encrypted for account '{}' of this wallet, make it the active account", + label + ), + None => { + "Could not decrypt slatepack with any provided index on the address derivation path" + .to_owned() + } + }; + Err(Error::SlatepackDecryption(msg)) } } diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index 9d7e505..1606a0e 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -240,6 +240,8 @@ where // field to report whether the transaction data is held. tx_log_entry.stored_tx = Some(format!("{}.grintx", slate.id)); } + // as update_tx_slate_state does + tx_log_entry.tx_slate_state = Some(slate.state.clone()); // If we added outputs in this step, we have to create OutputData here because 'batch' // takes the mutable ref and we can no longer call calc_commit_for_cache for output let added_outputs = if !step_added_outputs { diff --git a/tests/cmd_line_contract.rs b/tests/cmd_line_contract.rs index 0047909..af75662 100644 --- a/tests/cmd_line_contract.rs +++ b/tests/cmd_line_contract.rs @@ -210,6 +210,62 @@ fn contract_command_test_impl(test_dir: &str) -> Result<(), grin_wallet_controll ]; execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?; + // slatepack for another account needs --account + let arg_vec = vec!["grin-wallet", "-p", "password1", "account", "-c", "second"]; + execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?; + let mut second = None; + grin_wallet_controller::controller::owner_single_use( + wallet1.clone(), + mask1, + std::path::PathBuf::from(test_dir), + |api, m| { + api.set_active_account(m, "second")?; + second = Some(api.get_slatepack_address(m, 0)?.to_string()); + api.set_active_account(m, "default")?; + Ok(()) + }, + )?; + let second = second.unwrap(); + let arg_vec = vec![ + "grin-wallet", + "-p", + "password2", + "contract", + "new", + "--receive", + "1", + "--no-payjoin", + "--min_conf", + "1", + "--encrypt-for", + &second, + ]; + execute_command(&app, test_dir, "wallet2", &client2, arg_vec)?; + let file_name = only_slatepack(test_dir, "wallet2"); + let arg_vec = vec![ + "grin-wallet", + "-p", + "password1", + "contract", + "view", + "-i", + &file_name, + ]; + let err = execute_command(&app, test_dir, "wallet1", &client1, arg_vec).unwrap_err(); + assert!(err.to_string().contains("account 'second'"), "{}", err); + let arg_vec = vec![ + "grin-wallet", + "-p", + "password1", + "-a", + "second", + "contract", + "view", + "-i", + &file_name, + ]; + execute_command(&app, test_dir, "wallet1", &client1, arg_vec)?; + // A file that isn't there is reported, not panicked on let arg_vec = vec![ "grin-wallet",