From 0683a3a307d8cd4c43329d1763879d9ae9e7f3c0 Mon Sep 17 00:00:00 2001 From: wiesche Date: Tue, 15 Sep 2026 21:59:15 +0200 Subject: [PATCH] share proof amount and drop serde defaults --- controller/src/display.rs | 33 ++-------------- controller/tests/contract_srs.rs | 53 ++++++++++++++++++++++++++ libwallet/src/api_impl/owner.rs | 25 +++--------- libwallet/src/slate_versions/common.rs | 12 +----- libwallet/src/slate_versions/v4.rs | 38 ++++-------------- libwallet/src/slate_versions/v5.rs | 38 ++++-------------- libwallet/src/types.rs | 31 +++++++++++++++ 7 files changed, 109 insertions(+), 121 deletions(-) diff --git a/controller/src/display.rs b/controller/src/display.rs index 427078a..9c03823 100644 --- a/controller/src/display.rs +++ b/controller/src/display.rs @@ -594,11 +594,7 @@ pub fn payment_proof(tx: &TxLogEntry) -> Result<(), Error> { } None => "None".to_owned(), }; - let fee = match tx.fee { - Some(f) => f.fee(), // apply fee mask past HF4 - None => 0, - }; - let amount = core::amount_to_hr_string(proof_amount(tx, pp.proof_type.is_some(), fee), true); + let amount = core::amount_to_hr_string(tx.payment_proof_amount(), true); let sender_signature = match pp.sender_signature { Some(s) => { @@ -640,15 +636,6 @@ 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"); @@ -768,12 +755,11 @@ fn format_net_change(change: i64) -> String { #[cfg(test)] mod tests { - use super::{contract_view_table, format_net_change, proof_amount}; + use super::{contract_view_table, format_net_change}; 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, TxLogEntry, TxLogEntryType}; + use crate::libwallet::Slate; use crate::util::secp::pedersen::{Commitment, RangeProof}; fn table_value(table: &prettytable::Table, label: &str) -> String { @@ -797,19 +783,6 @@ 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_srs.rs b/controller/tests/contract_srs.rs index dd4215e..78cfe76 100644 --- a/controller/tests/contract_srs.rs +++ b/controller/tests/contract_srs.rs @@ -712,6 +712,59 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> assert_eq!(signed.state, SlateState::Standard3); } + // proof export of a proofless contract whose amount is below the sender's fee + let small_args = |net_change| ContractSetupArgsAPI { + selection_args: common::contract_selection_args(), + net_change, + ..Default::default() + }; + let receiver_fee = my_fee_contribution(0, 1, 1, 2)?.fee(); + let net = i64::try_from((receiver_fee + participant_fee) / 2).unwrap(); + let mut small = Slate::blank(0, false); + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + let args = ContractNewArgsAPI { + setup_args: small_args(Some(-net)), + ..Default::default() + }; + small = api.contract_new(m, &args)?; + Ok(()) + }, + )?; + wallet::controller::owner_single_use( + recv_wallet.clone(), + recv_mask, + PathBuf::from(test_dir), + |api, m| { + let mut args = small_args(Some(net)); + args.selection_args.use_inputs = None; + small = api.contract_sign(m, &small, &args)?; + Ok(()) + }, + )?; + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + small = api.contract_sign(m, &small, &small_args(None))?; + let tx_log = common::tx_log_for_slate(api, m, &small)?; + assert!(tx_log.fee.map(|f| f.fee()).unwrap_or(0) > tx_log.amount_debited); + assert!(matches!( + api.retrieve_payment_proof(m, false, None, Some(small.id)), + Err(libwallet::Error::PaymentProofRetrieval(_)) + )); + assert!(matches!( + api.retrieve_payment_proof_early(m, false, None, Some(small.id)), + Err(libwallet::Error::PaymentProofRetrieval(_)) + )); + Ok(()) + }, + )?; + // let logging finish stopper.store(false, Ordering::Relaxed); thread::sleep(Duration::from_millis(200)); diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index c2fd727..5d42527 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -529,7 +529,7 @@ where tx_id, tx_slate_id, )?; - let proof = match tx.payment_proof { + let proof = match tx.payment_proof.as_ref() { Some(p) => p, None => { return Err(Error::PaymentProofRetrieval( @@ -537,15 +537,7 @@ where )); } }; - let amount = if tx.amount_credited >= tx.amount_debited { - tx.amount_credited - tx.amount_debited - } else { - let fee = match tx.fee { - Some(f) => f.fee(), // apply fee mask past HF4 - None => 0, - }; - tx.amount_debited - tx.amount_credited - fee - }; + let amount = tx.payment_proof_amount(); let excess = match tx.kernel_excess { Some(e) => e, None => { @@ -620,16 +612,11 @@ where tx_id, tx_slate_id, )?; - // Contract tx logs store the agreed net change separately from the fee. - let amount = tx.amount_credited.abs_diff(tx.amount_debited); - let sender_public_nonce = tx - .payment_proof - .as_ref() - .and_then(|proof| proof.sender_public_nonce); - - let p = tx.payment_proof.ok_or_else(|| { + let p = tx.payment_proof.as_ref().ok_or_else(|| { Error::PaymentProofRetrieval("Transaction does not contain a payment proof".to_owned()) })?; + let amount = tx.payment_proof_amount(); + let sender_public_nonce = p.sender_public_nonce; let receiver_public_nonce = p.receiver_public_nonce.ok_or_else(|| { Error::PaymentProofRetrieval( "Early payment proof requires stored receiver public nonce".into(), @@ -657,7 +644,7 @@ where receiver_public_excess, sender_address: p.sender_address, timestamp: timestamp.timestamp(), - memo: p.memo, + memo: p.memo.clone(), promise_signature: p.receiver_signature, witness_data: None, }; diff --git a/libwallet/src/slate_versions/common.rs b/libwallet/src/slate_versions/common.rs index 4d918bf..307448c 100644 --- a/libwallet/src/slate_versions/common.rs +++ b/libwallet/src/slate_versions/common.rs @@ -59,16 +59,12 @@ pub struct ParticipantData { #[serde(with = "secp_ser::pubkey_serde")] pub nonce: PublicKey, /// Public partial signature - #[serde(default = "default_part_sig_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] #[serde(with = "secp_ser::option_sig_serde")] pub part: Option, } -fn default_part_sig_none() -> Option { - None -} - /// Input commitment or output with its range proof #[derive(Debug, Copy, Clone, Serialize, Deserialize)] pub struct Commits { @@ -85,7 +81,7 @@ pub struct Commits { /// A proof that the commitment is in the right range /// Only applies for transaction outputs #[serde(with = "ser::option_rangeproof_hex")] - #[serde(default = "default_range_proof")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub p: Option, } @@ -133,10 +129,6 @@ pub fn sig_is_blank(s: &secp::Signature) -> bool { true } -fn default_range_proof() -> Option { - None -} - /// A mining node requests new coinbase via the foreign api every time a new candidate block is built. #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Coinbase { diff --git a/libwallet/src/slate_versions/v4.rs b/libwallet/src/slate_versions/v4.rs index 2325404..d76885a 100644 --- a/libwallet/src/slate_versions/v4.rs +++ b/libwallet/src/slate_versions/v4.rs @@ -95,7 +95,7 @@ pub struct SlateV4 { /// base amount (excluding fee) #[serde(with = "secp_ser::string_or_u64")] #[serde(skip_serializing_if = "u64_is_blank")] - #[serde(default = "default_u64_0")] + #[serde(default)] pub amt: u64, /// fee #[serde(skip_serializing_if = "fee_is_zero")] @@ -103,13 +103,13 @@ pub struct SlateV4 { pub fee: FeeFields, /// kernel features, if any #[serde(skip_serializing_if = "u8_is_blank")] - #[serde(default = "default_u8_0")] + #[serde(default)] pub feat: u8, /// TTL, the block height at which wallets /// should refuse to process the transaction and unlock all #[serde(with = "secp_ser::string_or_u64")] #[serde(skip_serializing_if = "u64_is_blank")] - #[serde(default = "default_u64_0")] + #[serde(default)] pub ttl: u64, // Structs always required /// Participant data, each participant in the transaction will @@ -118,24 +118,20 @@ pub struct SlateV4 { pub sigs: Vec, // Situational, but required at some point in the tx /// Inputs/Output commits added to slate - #[serde(default = "default_coms_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub coms: Option>, // Optional Structs /// Payment Proof - #[serde(default = "default_payment_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub proof: Option, /// Kernel features arguments - #[serde(default = "default_kernel_features_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub feat_args: Option, } -fn default_payment_none() -> Option { - None -} - fn default_offset_zero() -> BlindingFactor { BlindingFactor::zero() } @@ -144,14 +140,6 @@ fn offset_is_zero(o: &BlindingFactor) -> bool { *o == BlindingFactor::zero() } -fn default_coms_none() -> Option> { - None -} - -fn default_u64_0() -> u64 { - 0 -} - fn num_parts_is_2(n: &u8) -> bool { *n == 2 } @@ -160,10 +148,6 @@ fn default_num_participants_2() -> u8 { 2 } -fn default_kernel_features_none() -> Option { - None -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct VersionCompatInfoV4 { /// The current version of the slate format @@ -178,24 +162,16 @@ pub struct PaymentInfoV4 { pub saddr: DalekPublicKey, #[serde(with = "ser::dalek_pubkey_serde")] pub raddr: DalekPublicKey, - #[serde(default = "default_receiver_signature_none")] + #[serde(default)] #[serde(with = "ser::option_dalek_sig_serde")] #[serde(skip_serializing_if = "Option::is_none")] pub rsig: Option, } -fn default_receiver_signature_none() -> Option { - None -} - fn u64_is_blank(u: &u64) -> bool { *u == 0 } -fn default_u8_0() -> u8 { - 0 -} - fn u8_is_blank(u: &u8) -> bool { *u == 0 } diff --git a/libwallet/src/slate_versions/v5.rs b/libwallet/src/slate_versions/v5.rs index d5860ba..0c32165 100644 --- a/libwallet/src/slate_versions/v5.rs +++ b/libwallet/src/slate_versions/v5.rs @@ -73,7 +73,7 @@ pub struct SlateV5 { /// base amount (excluding fee) #[serde(with = "secp_ser::string_or_u64")] #[serde(skip_serializing_if = "u64_is_blank")] - #[serde(default = "default_u64_0")] + #[serde(default)] pub amt: u64, /// fee #[serde(skip_serializing_if = "fee_is_zero")] @@ -81,13 +81,13 @@ pub struct SlateV5 { pub fee: FeeFields, /// kernel features, if any #[serde(skip_serializing_if = "u8_is_blank")] - #[serde(default = "default_u8_0")] + #[serde(default)] pub feat: u8, /// TTL, the block height at which wallets /// should refuse to process the transaction and unlock all #[serde(with = "secp_ser::string_or_u64")] #[serde(skip_serializing_if = "u64_is_blank")] - #[serde(default = "default_u64_0")] + #[serde(default)] pub ttl: u64, // Structs always required /// Participant data, each participant in the transaction will @@ -96,24 +96,20 @@ pub struct SlateV5 { pub sigs: Vec, // Situational, but required at some point in the tx /// Inputs/Output commits added to slate - #[serde(default = "default_coms_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub coms: Option>, // Optional Structs /// Payment Proof - #[serde(default = "default_payment_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub proof: Option, /// Kernel features arguments - #[serde(default = "default_kernel_features_none")] + #[serde(default)] #[serde(skip_serializing_if = "Option::is_none")] pub feat_args: Option, } -fn default_payment_none() -> Option { - None -} - fn default_offset_zero() -> BlindingFactor { BlindingFactor::zero() } @@ -122,14 +118,6 @@ fn offset_is_zero(o: &BlindingFactor) -> bool { *o == BlindingFactor::zero() } -fn default_coms_none() -> Option> { - None -} - -fn default_u64_0() -> u64 { - 0 -} - fn num_parts_is_2(n: &u8) -> bool { *n == 2 } @@ -138,10 +126,6 @@ fn default_num_participants_2() -> u8 { 2 } -fn default_kernel_features_none() -> Option { - None -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct VersionCompatInfoV5 { /// The current version of the slate format @@ -163,7 +147,7 @@ pub struct PaymentInfoV5 { #[serde_as(as = "Option>")] #[serde(default, skip_serializing_if = "Option::is_none")] pub ts: Option>, - #[serde(default = "default_promise_signature_none")] + #[serde(default)] #[serde(with = "ser::option_dalek_sig_serde")] #[serde(skip_serializing_if = "Option::is_none")] pub psig: Option, @@ -171,18 +155,10 @@ pub struct PaymentInfoV5 { pub memo: Option, } -fn default_promise_signature_none() -> Option { - None -} - fn u64_is_blank(u: &u64) -> bool { *u == 0 } -fn default_u8_0() -> u8 { - 0 -} - fn u8_is_blank(u: &u8) -> bool { *u == 0 } diff --git a/libwallet/src/types.rs b/libwallet/src/types.rs index 1f4059e..47446ec 100644 --- a/libwallet/src/types.rs +++ b/libwallet/src/types.rs @@ -769,6 +769,20 @@ impl TxLogEntry { pub fn update_confirmation_ts(&mut self) { self.confirmation_ts = Some(Utc::now()); } + + /// Amount covered by the stored payment proof; early (contract) proofs don't subtract the fee + pub fn payment_proof_amount(&self) -> u64 { + let early = self + .payment_proof + .as_ref() + .is_some_and(|p| p.proof_type.is_some()); + if early || self.amount_credited >= self.amount_debited { + self.amount_credited.abs_diff(self.amount_debited) + } else { + let fee = self.fee.map(|f| f.fee()).unwrap_or(0); // apply fee mask past HF4 + self.amount_debited - self.amount_credited - fee + } + } } /// Payment proof information. Differs from what is sent via @@ -1073,6 +1087,23 @@ mod tests { dur: Option, } + #[test] + fn payment_proof_amount() { + let key = DalekSecretKey::from_bytes(&[1u8; 32]).verifying_key(); + let mut tx = TxLogEntry::new(Identifier::zero(), TxLogEntryType::TxSent, 0); + tx.fee = Some(FeeFields::new(0, 23_000_000).unwrap()); + tx.payment_proof = Some(StoredProofInfo::new(key, None, key, 0, None)); + // legacy send + tx.amount_debited = 1_500_000_000; + tx.amount_credited = 477_000_000; + assert_eq!(tx.payment_proof_amount(), 1_000_000_000); + // contract send + tx.amount_debited = 200_000_000; + tx.amount_credited = 0; + tx.payment_proof.as_mut().unwrap().proof_type = Some(1); + assert_eq!(tx.payment_proof_amount(), 200_000_000); + } + #[test] fn duration_serde() { let some = TestSer {