From c613d82dafef0b0564ef8a78ca70d1070b21a3b4 Mon Sep 17 00:00:00 2001 From: wiesche Date: Mon, 7 Sep 2026 18:53:36 +0200 Subject: [PATCH] respect slate version --- api/src/owner_rpc.rs | 72 ++++++++++++++++++------ controller/tests/slatepack.rs | 26 ++++++--- libwallet/src/api_impl/owner.rs | 14 ++++- libwallet/src/api_impl/types.rs | 6 +- libwallet/src/contract/actions/README.md | 1 - libwallet/src/contract/actions/new.rs | 2 + libwallet/src/internal/tx.rs | 2 + libwallet/src/slate_versions/mod.rs | 27 ++++++--- libwallet/src/slatepack/packer.rs | 4 +- 9 files changed, 110 insertions(+), 44 deletions(-) diff --git a/api/src/owner_rpc.rs b/api/src/owner_rpc.rs index 5ef472b..8baac99 100644 --- a/api/src/owner_rpc.rs +++ b/api/src/owner_rpc.rs @@ -2119,6 +2119,11 @@ pub trait OwnerRpc { ) -> Result; } +fn versioned_output(slate: Slate) -> Result { + let version = SlateVersion::output_for(&slate)?; + VersionedSlate::into_version(slate, version) +} + impl OwnerRpc for Owner where L: WalletLCProvider<'static, C, K>, @@ -2216,8 +2221,7 @@ where fn init_send_tx(&self, token: Token, args: InitTxArgs) -> Result { let slate = Owner::init_send_tx(self, (&token.keychain_mask).as_ref(), args)?; - let version = SlateVersion::V4; - VersionedSlate::into_version(slate, version) + versioned_output(slate) } fn contract_new( @@ -2238,20 +2242,13 @@ where in_slate: VersionedSlate, args: ContractSetupArgsAPI, ) -> Result { - let in_version = in_slate.version(); let slate = Owner::contract_sign( self, (&token.keychain_mask).as_ref(), &Slate::from(in_slate), &args, )?; - // Keep the version the counterparty sent us, since they can clearly read it, but - // never below what the slate needs to represent its payment proof. - let version = match SlateVersion::lowest_for(&slate) { - SlateVersion::V5 => SlateVersion::V5, - SlateVersion::V4 => in_version, - }; - VersionedSlate::into_version(slate, version) + versioned_output(slate) } fn contract_view(&self, token: Token, slate: VersionedSlate) -> Result { @@ -2278,8 +2275,7 @@ where args: IssueInvoiceTxArgs, ) -> Result { let slate = Owner::issue_invoice_tx(self, (&token.keychain_mask).as_ref(), args)?; - let version = SlateVersion::V4; - VersionedSlate::into_version(slate, version) + versioned_output(slate) } fn process_invoice_tx( @@ -2294,8 +2290,7 @@ where &Slate::from(in_slate), args, )?; - let version = SlateVersion::V4; - VersionedSlate::into_version(out_slate, version) + versioned_output(out_slate) } fn tx_lock_outputs(&self, token: Token, in_slate: VersionedSlate) -> Result<(), Error> { @@ -2312,8 +2307,7 @@ where (&token.keychain_mask).as_ref(), &Slate::from(in_slate), )?; - let version = SlateVersion::V4; - VersionedSlate::into_version(out_slate, version) + versioned_output(out_slate) } fn post_tx(&self, token: Token, slate: VersionedSlate, fluff: bool) -> Result<(), Error> { @@ -2538,8 +2532,7 @@ where secret_indices, )?; // Keep the decoded version; lowest_for would downgrade a plain V5 slate - let version = SlateVersion::try_from(slate.version_info.version)?; - VersionedSlate::into_version(slate, version) + versioned_output(slate) } fn decode_slatepack_message( @@ -2900,6 +2893,18 @@ mod tests { use grin_wallet_util::byte_ser; use tempfile::tempdir; + #[test] + fn output_keeps_version() { + for version in [4, 5] { + let mut slate = Slate::blank(2, false); + slate.version_info.version = version; + assert_eq!( + versioned_output(slate).unwrap().version(), + SlateVersion::try_from(version).unwrap() + ); + } + } + #[test] fn slatepack_keeps_v5() { let slate = VersionedSlate::into_version(Slate::blank(2, false), SlateVersion::V5).unwrap(); @@ -2932,4 +2937,35 @@ mod tests { assert_eq!(response["result"]["Ok"]["ver"], "5:3"); } + + #[test] + fn invoice_uses_target_version() { + let request = serde_json::json!({ + "jsonrpc": "2.0", + "method": "issue_invoice_tx", + "params": { + "token": "d202964900000000d302964900000000d402964900000000d502964900000000", + "args": { + "amount": "6000000000", + "dest_acct_name": null, + "target_slate_version": 5 + } + }, + "id": 1 + }); + let dir = tempdir().unwrap(); + let response = run_doctest_owner( + request, + dir.path().to_str().unwrap(), + 0, + false, + false, + false, + false, + ) + .unwrap() + .unwrap(); + + assert_eq!(response["result"]["Ok"]["ver"], "5:1"); + } } diff --git a/controller/tests/slatepack.rs b/controller/tests/slatepack.rs index 74823ca..f4d44ac 100644 --- a/controller/tests/slatepack.rs +++ b/controller/tests/slatepack.rs @@ -84,7 +84,9 @@ fn slatepack_exchange_test_impl( use_bin: bool, use_armored: bool, use_encryption: bool, + target_slate_version: Option, ) -> Result<(), libwallet::Error> { + let expected_slate_version = target_slate_version.unwrap_or(4); // Create a new proxy to simulate server and wallet responses let mut wallet_proxy = create_wallet_proxy(test_dir); let chain = wallet_proxy.chain.clone(); @@ -236,6 +238,7 @@ fn slatepack_exchange_test_impl( max_outputs: 500, num_change_outputs: 1, selection_strategy_is_use_all: true, + target_slate_version, ..Default::default() }; let slate = api.init_send_tx(m, args)?; @@ -261,6 +264,7 @@ fn slatepack_exchange_test_impl( let (mut slatepack, mut slate) = slate_from_packed(&send_file, use_armored, (&dec_key_2).as_ref())?; + assert_eq!(slate.version_info.version, expected_slate_version); // wallet 2 receives file, completes, sends file back wallet::controller::foreign_single_use( @@ -293,7 +297,9 @@ fn slatepack_exchange_test_impl( |api, m| { let (_, mut slate) = slate_from_packed(&receive_file, use_armored, (&dec_key_1).as_ref())?; + assert_eq!(slate.version_info.version, expected_slate_version); slate = api.finalize_tx(m, &slate)?; + assert_eq!(slate.version_info.version, expected_slate_version); // Output final file for reference output_slatepack(&slate, &final_file, use_armored, use_bin, None, vec![])?; api.post_tx(m, &slate, false)?; @@ -357,6 +363,7 @@ fn slatepack_exchange_test_impl( |api, m| { let args = IssueInvoiceTxArgs { amount: 1000000000, + target_slate_version, ..Default::default() }; slate = api.issue_invoice_tx(m, args)?; @@ -389,6 +396,7 @@ fn slatepack_exchange_test_impl( let res = slate_from_packed(&send_file, use_armored, (&dec_key_1).as_ref())?; slatepack = res.0; slate = res.1; + assert_eq!(slate.version_info.version, expected_slate_version); slate = api.process_invoice_tx(m, &slate, args)?; api.tx_lock_outputs(m, &slate)?; output_slatepack( @@ -413,7 +421,9 @@ fn slatepack_exchange_test_impl( // Wallet 2 receives the invoice transaction let res = slate_from_packed(&receive_file, use_armored, (&dec_key_2).as_ref())?; slate = res.1; + assert_eq!(slate.version_info.version, expected_slate_version); slate = api.finalize_tx(&slate, false)?; + assert_eq!(slate.version_info.version, expected_slate_version); output_slatepack(&slate, &final_file, use_armored, use_bin, None, vec![])?; Ok(()) }, @@ -707,11 +717,11 @@ fn slatepack_address_validation(test_dir: &'static str) -> Result<(), libwallet: } #[test] -fn slatepack_exchange_json() { - let test_dir = "test_output/slatepack_exchange_json"; +fn slatepack_exchange_json_v5() { + let test_dir = "test_output/slatepack_exchange_json_v5"; setup(test_dir); // Json output - if let Err(e) = slatepack_exchange_test_impl(test_dir, false, false, false) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, false, false, false, Some(5)) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); @@ -722,7 +732,7 @@ fn slatepack_exchange_bin() { let test_dir = "test_output/slatepack_exchange_bin"; setup(test_dir); // Bin output - if let Err(e) = slatepack_exchange_test_impl(test_dir, true, false, false) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, true, false, false, None) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); @@ -733,7 +743,7 @@ fn slatepack_exchange_armored() { let test_dir = "test_output/slatepack_exchange_armored"; setup(test_dir); // Bin output - if let Err(e) = slatepack_exchange_test_impl(test_dir, true, true, true) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, true, true, true, None) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); @@ -744,7 +754,7 @@ fn slatepack_exchange_json_enc() { let test_dir = "test_output/slatepack_exchange_json_enc"; setup(test_dir); // Json output - if let Err(e) = slatepack_exchange_test_impl(test_dir, false, false, true) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, false, false, true, None) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); @@ -755,7 +765,7 @@ fn slatepack_exchange_bin_enc() { let test_dir = "test_output/slatepack_exchange_bin_enc"; setup(test_dir); // Bin output - if let Err(e) = slatepack_exchange_test_impl(test_dir, true, false, true) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, true, false, true, None) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); @@ -766,7 +776,7 @@ fn slatepack_exchange_armored_enc() { let test_dir = "test_output/slatepack_exchange_armored_enc"; setup(test_dir); // Bin output - if let Err(e) = slatepack_exchange_test_impl(test_dir, true, true, true) { + if let Err(e) = slatepack_exchange_test_impl(test_dir, true, true, true, None) { panic!("Libwallet Error: {}", e); } clean_output_dir(test_dir); diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index f1df0a7..7d2f3d7 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -42,9 +42,9 @@ use crate::{ address, contract, mwixnet::{create_onion, ComSignature, Hop, MixnetReqCreationParams, SwapReq}, wallet_lock, BuiltOutput, Error, InitTxArgs, IssueInvoiceTxArgs, NodeHeightResult, - OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, ScannedBlockInfo, Slatepack, - SlatepackAddress, Slatepacker, SlatepackerArgs, TxLogEntryType, ViewWallet, WalletBackend, - WalletInitStatus, WalletInst, WalletLCProvider, + OutputCommitMapping, PaymentProof, RetrieveTxQueryArgs, ScannedBlockInfo, SlateVersion, + Slatepack, SlatepackAddress, Slatepacker, SlatepackerArgs, TxLogEntryType, ViewWallet, + WalletBackend, WalletInitStatus, WalletInst, WalletLCProvider, }; use ed25519_dalek::SigningKey as DalekSecretKey; use ed25519_dalek::Verifier; @@ -675,6 +675,10 @@ where C: NodeClient, K: Keychain, { + if let Some(v) = args.target_slate_version { + SlateVersion::try_from(v)?; + } + let payment_proof_address = if let Some(a) = &args.payment_proof_recipient_address { if a.valid_network() { Some(a) @@ -801,6 +805,10 @@ where C: NodeClient, K: Keychain, { + if let Some(v) = args.target_slate_version { + SlateVersion::try_from(v)?; + } + let parent_key_id = match args.dest_acct_name { Some(d) => { let pm = w.get_acct_path(d)?; diff --git a/libwallet/src/api_impl/types.rs b/libwallet/src/api_impl/types.rs index 0575fab..4e59873 100644 --- a/libwallet/src/api_impl/types.rs +++ b/libwallet/src/api_impl/types.rs @@ -72,8 +72,7 @@ pub struct InitTxArgs { #[serde(default = "default_refresh_outputs_from_node")] pub refresh_outputs_from_node: bool, /// Optionally set the output target slate version (acceptable - /// down to the minimum slate version compatible with the current. If `None` the slate - /// is generated with the latest version. + /// down to the minimum slate version compatible with the current. If `None`, V4 is used. pub target_slate_version: Option, /// Number of blocks from current after which TX should be ignored #[serde(with = "secp_ser::opt_string_or_u64")] @@ -142,8 +141,7 @@ pub struct IssueInvoiceTxArgs { #[serde(with = "secp_ser::string_or_u64")] pub amount: u64, /// Optionally set the output target slate version (acceptable - /// down to the minimum slate version compatible with the current. If `None` the slate - /// is generated with the latest version. + /// down to the minimum slate version compatible with the current. If `None`, V4 is used. pub target_slate_version: Option, } diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index cbc3c55..08d2817 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -46,7 +46,6 @@ Separating side effects until the 'save_step' part would make these functions mu - Preserve kernel features and their arguments across V4 and V5; see #793. - Choose the sender derivation path explicitly and keep encrypted outgoing files identifiable locally. - - Decide whether `target_slate_version` should control Slatepack output or be removed. - Add contract history, lookup and transport. - Decide whether the foreign JSON-RPC API should expose `new` and `sign`. - Add full API and RPC examples once the contract interface is stable. diff --git a/libwallet/src/contract/actions/new.rs b/libwallet/src/contract/actions/new.rs index 37cfbfb..318fd1b 100644 --- a/libwallet/src/contract/actions/new.rs +++ b/libwallet/src/contract/actions/new.rs @@ -70,6 +70,8 @@ where // Initialize a new contract (if net_change is positive, I'm the receiver meaning this is invoice flow) let num_participants = setup_args.num_participants; let mut slate = Slate::blank(num_participants, net_change > 0); + // Contracts start with V4 and move to V5 when their proof data requires it + slate.version_info.version = 4; // Use a caller-supplied id when given, so a retried creation reuses the same context. let mut reused_context = false; if let Some(id) = slate_id { diff --git a/libwallet/src/internal/tx.rs b/libwallet/src/internal/tx.rs index 79cd1a3..dcebdbb 100644 --- a/libwallet/src/internal/tx.rs +++ b/libwallet/src/internal/tx.rs @@ -57,6 +57,8 @@ where { let current_height = wallet.w2n_client().get_chain_tip()?.0; let mut slate = Slate::blank(num_participants, is_invoice); + // Start with V4 for compatibility unless the caller requests a newer version + slate.version_info.version = 4; if let Some(b) = ttl_blocks { slate.ttl_cutoff_height = current_height + b; } diff --git a/libwallet/src/slate_versions/mod.rs b/libwallet/src/slate_versions/mod.rs index 1d6a9ff..6e87881 100644 --- a/libwallet/src/slate_versions/mod.rs +++ b/libwallet/src/slate_versions/mod.rs @@ -65,15 +65,23 @@ impl TryFrom for SlateVersion { } impl SlateVersion { - /// The lowest version that can represent this slate without losing data, so a slate - /// is only sent as V5 when it has to be. V4 carries the payment proof's sender and - /// receiver addresses and its signature, but not the proof type, timestamp or memo. + /// Use V5 only when the slate has fields that V4 cannot store + /// V4 does not support the proof type, timestamp or memo pub fn lowest_for(slate: &Slate) -> SlateVersion { match &slate.payment_proof { Some(p) if p.requires_v5() => SlateVersion::V5, _ => SlateVersion::V4, } } + + /// The requested output version, raised when the slate needs a newer version + pub fn output_for(slate: &Slate) -> Result { + let requested = SlateVersion::try_from(slate.version_info.version)?; + Ok(match SlateVersion::lowest_for(slate) { + SlateVersion::V5 => SlateVersion::V5, + SlateVersion::V4 => requested, + }) + } } #[derive(Debug, Serialize, Deserialize)] @@ -318,7 +326,7 @@ pub mod tests { } #[test] - fn create_slatepack_emits_lowest_version() -> Result<(), Error> { + fn create_slatepack_uses_output_version() -> Result<(), Error> { set_local_chain_type(ChainTypes::Mainnet); let packer = Slatepacker::new(SlatepackerArgs { sender: None, @@ -334,16 +342,20 @@ pub mod tests { assert_eq!(recovered_v5.version_info.version, 5); assert!(recovered_v5.payment_proof.unwrap().memo.is_some()); - // Without a proof, V4 is sufficient and is emitted for interoperability with V4 peers. + // A plain slate keeps its requested version. let mut slate_no_proof = populate_test_slate()?; slate_no_proof.payment_proof = None; + let recovered_v5 = packer.get_slate(&packer.create_slatepack(&slate_no_proof)?)?; + assert_eq!(recovered_v5.version_info.version, 5); + + slate_no_proof.version_info.version = 4; let recovered_v4 = packer.get_slate(&packer.create_slatepack(&slate_no_proof)?)?; assert_eq!(recovered_v4.version_info.version, 4); - // A legacy send proof uses no V5 field. It must stay V4 so V4-only wallets can read it, - // and the proof has to survive the round trip intact. + // A legacy send proof also fits in V4 and survives the round trip intact. let mut slate_legacy_proof = populate_test_slate()?; use_legacy_proof(&mut slate_legacy_proof); + slate_legacy_proof.version_info.version = 4; let proof = slate_legacy_proof.payment_proof.clone().unwrap(); let recovered_legacy = packer.get_slate(&packer.create_slatepack(&slate_legacy_proof)?)?; assert_eq!(recovered_legacy.version_info.version, 4); @@ -358,6 +370,7 @@ pub mod tests { proof.timestamp = None; proof.memo = None; proof.promise_signature = None; + partial_proof.version_info.version = 4; let recovered = packer.get_slate(&packer.create_slatepack(&partial_proof)?)?; assert_eq!(recovered.version_info.version, 5); let proof = recovered.payment_proof.unwrap(); diff --git a/libwallet/src/slatepack/packer.rs b/libwallet/src/slatepack/packer.rs index 27dda49..a7b4c97 100644 --- a/libwallet/src/slatepack/packer.rs +++ b/libwallet/src/slatepack/packer.rs @@ -94,9 +94,7 @@ impl<'a> Slatepacker<'a> { /// Create slatepack from slate and args pub fn create_slatepack(&self, slate: &Slate) -> Result { - // Emit the lowest version that can represent this slate, for maximum - // interoperability with V4-only wallets. - let version = SlateVersion::lowest_for(slate); + let version = SlateVersion::output_for(slate)?; let out_slate = VersionedSlate::into_version(slate.clone(), version)?; let bin_slate = VersionedBinSlate::try_from(out_slate).map_err(|_| Error::SlatepackSer)?; let mut slatepack = Slatepack::default();