diff --git a/controller/tests/contract_self_spend_cancel.rs b/controller/tests/contract_self_spend_cancel.rs index 5f732f3..6612580 100644 --- a/controller/tests/contract_self_spend_cancel.rs +++ b/controller/tests/contract_self_spend_cancel.rs @@ -97,6 +97,130 @@ fn contract_self_spend_cancel_impl(test_dir: &'static str) -> Result<(), libwall Ok(()) } +/// Find the stored transaction a signed contract wrote +fn find_stored_tx(test_dir: &str) -> std::path::PathBuf { + fn walk(dir: &std::path::Path, out: &mut Vec) { + if let Ok(entries) = std::fs::read_dir(dir) { + for e in entries.filter_map(|e| e.ok()) { + let p = e.path(); + if p.is_dir() { + walk(&p, out); + } else if p.extension().map(|x| x == "grintx").unwrap_or(false) { + out.push(p); + } + } + } + } + let mut found = vec![]; + walk(std::path::Path::new(test_dir), &mut found); + assert_eq!(found.len(), 1, "expected one stored tx under {}", test_dir); + found.pop().unwrap() +} + +/// save_step writes the signed transaction to a file after the wallet state batch has +/// committed, so that write can fail with the tx log entry and the input locks already +/// persisted. The documented way out is to cancel, which does not read the stored tx; +/// this checks that holds by removing the file and cancelling. +fn contract_self_spend_cancel_missing_stored_tx_impl( + test_dir: &'static str, +) -> Result<(), libwallet::Error> { + let (wallets, _chain, stopper, _bh) = + create_wallets(vec![vec![("default", 4)]], test_dir).unwrap(); + let send_wallet = wallets[0].0.clone(); + let send_mask = wallets[0].1.as_ref(); + + let mut slate = Slate::blank(0, true); + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + let args = &ContractNewArgsAPI { + setup_args: ContractSetupArgsAPI { + net_change: Some(0), + num_participants: 1, + ..Default::default() + }, + ..Default::default() + }; + slate = api.contract_new(m, args)?; + Ok(()) + }, + )?; + + // Signing a self-spend completes the slate, so the transaction is stored + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + let args = &ContractSetupArgsAPI { + ..Default::default() + }; + slate = api.contract_sign(m, &slate, args)?; + Ok(()) + }, + )?; + assert_eq!(slate.state, SlateState::Standard2); + + // Signing locked our inputs, so the cancel below has something to release, and the + // entry records where the transaction was stored, as a standard send does + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + let (_, info) = api.retrieve_summary_info(m, true, 1)?; + assert!(info.amount_locked > 0); + let (_, txs) = api.retrieve_txs(m, true, None, Some(slate.id), None)?; + assert_eq!( + txs.last().unwrap().stored_tx, + Some(format!("{}.grintx", slate.id)) + ); + Ok(()) + }, + )?; + + // Stand in for the file write having failed. The transaction has not been posted: + // contract_sign returns before the caller broadcasts. + let stored = find_stored_tx(test_dir); + std::fs::remove_file(&stored).unwrap(); + + // Cancelling still releases the inputs + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + api.cancel_tx(m, None, Some(slate.id))?; + Ok(()) + }, + )?; + + wallet::controller::owner_single_use( + send_wallet.clone(), + send_mask, + PathBuf::from(test_dir), + |api, m| { + let query_args = libwallet::RetrieveTxQueryArgs { + exclude_cancelled: Some(false), + ..Default::default() + }; + let (_, txs) = api.retrieve_txs(m, true, None, None, Some(query_args))?; + let tx_log = txs.last().unwrap(); + assert_eq!(tx_log.tx_type, TxLogEntryType::TxSelfSpendCancelled); + // The spendable balance is back to the mined total + let (_, info) = api.retrieve_summary_info(m, true, 1)?; + assert_eq!(info.amount_locked, 0); + Ok(()) + }, + )?; + + stopper.store(false, Ordering::Relaxed); + thread::sleep(Duration::from_millis(200)); + Ok(()) +} + #[test] fn wallet_contract_self_spend_cancel() -> Result<(), libwallet::Error> { let test_dir = "test_output/contract_self_spend_cancel"; @@ -105,3 +229,12 @@ fn wallet_contract_self_spend_cancel() -> Result<(), libwallet::Error> { clean_output_dir(test_dir); Ok(()) } + +#[test] +fn wallet_contract_self_spend_cancel_missing_stored_tx() -> Result<(), libwallet::Error> { + let test_dir = "test_output/contract_self_spend_cancel_missing_stored_tx"; + setup(test_dir); + contract_self_spend_cancel_missing_stored_tx_impl(test_dir)?; + clean_output_dir(test_dir); + Ok(()) +} diff --git a/libwallet/src/contract/utils.rs b/libwallet/src/contract/utils.rs index 439cd0d..b5eaf48 100644 --- a/libwallet/src/contract/utils.rs +++ b/libwallet/src/contract/utils.rs @@ -44,8 +44,7 @@ pub fn create_tx_log_entry( } }; let mut t = TxLogEntry::new(parent_key_id.clone(), log_type, log_id); - // stored_tx on the entry is left unset; contracts persist the tx separately via - // store_tx in save_step. + // stored_tx is set in save_step, once we have signed and the transaction is written. t.tx_slate_id = Some(slate.id); if net_change > 0 { @@ -223,6 +222,10 @@ where // Update TxLogEntry if we have signed the contract (we have data about the kernel) if is_signed { update_tx_log_entry(w, keychain_mask, &slate, &context, &mut tx_log_entry)?; + // Record where the transaction is stored, as internal::selection does for a + // standard send. store_tx below writes it under this name, and 'txs' reads the + // field to report whether the transaction data is held. + tx_log_entry.stored_tx = Some(format!("{}.grintx", slate.id)); } // 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 @@ -308,6 +311,15 @@ where // Store the signed transaction only after the wallet-state batch has committed (and // the signing context is confirmed gone), so a DB failure can't leave the stored tx // out of sync with wallet state. Matches the core convention (internal::selection). + // + // store_tx writes a file outside LMDB, so this cannot be part of the batch above and + // a window remains: the write can fail with the tx log entry and the input locks + // already committed. Making it atomic would mean holding stored transactions in the + // database, which is how every transaction in the wallet is kept, not just contracts. + // Until then the way out is to cancel the transaction, which releases the inputs + // without reading the stored tx. Nothing has been broadcast at this point: the error + // returns to the caller before it posts. Covered by + // wallet_contract_self_spend_cancel_missing_stored_tx. if is_signed { w.store_tx(&format!("{}", slate.id), slate.tx_or_err()?)?; }