Detect reverted transactions (#355)

* Detect reverted transactions

* Add reverted amount to funds display

* Support cancelling reverted txs

* Add reverted_after field for reverted transactions

* Update grin dependency to master branch

* Panic on failed test cleanup

* Only delete test dir if it exists

* Stop wallet proxy in accounts test

* Stop proxy thread in all controller tests

* Typo

* Add sleep after revert test

* Longer sleep in revert tests

api/src/owner_rpc.rs

@@ -270,6 +270,7 @@ pub trait OwnerRpc {
 			  "ttl_cutoff_height": null,
 			  "tx_slate_id": null,
 			  "payment_proof": null,
+			  "reverted_after": null,
 			  "tx_type": "ConfirmedCoinbase"
 			},
 			{
@@ -289,6 +290,7 @@ pub trait OwnerRpc {
 			  "stored_tx": null,
 			  "ttl_cutoff_height": null,
 			  "payment_proof": null,
+			  "reverted_after": null,
 			  "tx_slate_id": null,
 			  "tx_type": "ConfirmedCoinbase"
 			}
@@ -340,6 +342,7 @@ pub trait OwnerRpc {
 					"amount_currently_spendable": "60000000000",
 					"amount_immature": "180000000000",
 					"amount_locked": "0",
+					"amount_reverted": "0",
 					"last_confirmed_height": "4",
 					"minimum_confirmations": "1",
 					"total": "240000000000"

controller/src/display.rs

@@ -304,6 +304,12 @@ pub fn info(
 			bFG->"Confirmed Total",
 			FG->amount_to_hr_string(wallet_info.total, false)
 		]);
+		if wallet_info.amount_reverted > 0 {
+			table.add_row(row![
+				Fr->format!("Reverted"),
+				Fr->amount_to_hr_string(wallet_info.amount_reverted, false)
+			]);
+		}
 		// Only dispay "Immature Coinbase" if we have related outputs in the wallet.
 		// This row just introduces confusion if the wallet does not receive coinbase rewards.
 		if wallet_info.amount_immature > 0 {
@@ -337,6 +343,12 @@ pub fn info(
 			bFG->"Total",
 			FG->amount_to_hr_string(wallet_info.total, false)
 		]);
+		if wallet_info.amount_reverted > 0 {
+			table.add_row(row![
+				Fr->format!("Reverted"),
+				Fr->amount_to_hr_string(wallet_info.amount_reverted, false)
+			]);
+		}
 		// Only dispay "Immature Coinbase" if we have related outputs in the wallet.
 		// This row just introduces confusion if the wallet does not receive coinbase rewards.
 		if wallet_info.amount_immature > 0 {

controller/tests/revert.rs

@@ -0,0 +1,379 @@
+// Copyright 2020 The Grin Developers
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#[macro_use]
+mod common;
+
+use common::{clean_output_dir, create_wallet_proxy, setup};
+use grin_wallet_controller::controller::owner_single_use as owner;
+use grin_wallet_impls::test_framework::*;
+use grin_wallet_impls::{DefaultLCProvider, PathToSlate, SlatePutter};
+use grin_wallet_libwallet as libwallet;
+use grin_wallet_libwallet::api_impl::types::InitTxArgs;
+use grin_wallet_libwallet::WalletInst;
+use grin_wallet_util::grin_chain as chain;
+use grin_wallet_util::grin_core as core;
+use grin_wallet_util::grin_core::core::hash::Hashed;
+use grin_wallet_util::grin_core::core::Transaction;
+use grin_wallet_util::grin_core::global;
+use grin_wallet_util::grin_keychain::ExtKeychain;
+use grin_wallet_util::grin_util::secp::key::SecretKey;
+use grin_wallet_util::grin_util::Mutex;
+use log::error;
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::Arc;
+use std::thread;
+use std::time::Duration;
+
+type Wallet = Arc<
+	Mutex<
+		Box<
+			dyn WalletInst<
+				'static,
+				DefaultLCProvider<'static, LocalWalletClient, ExtKeychain>,
+				LocalWalletClient,
+				ExtKeychain,
+			>,
+		>,
+	>,
+>;
+
+fn revert(
+	test_dir: &'static str,
+) -> Result<
+	(
+		Arc<chain::Chain>,
+		Arc<AtomicBool>,
+		u64,
+		u64,
+		Transaction,
+		Wallet,
+		Option<SecretKey>,
+		Wallet,
+		Option<SecretKey>,
+	),
+	libwallet::Error,
+> {
+	let mut wallet_proxy = create_wallet_proxy(test_dir);
+	let stopper = wallet_proxy.running.clone();
+	let chain = wallet_proxy.chain.clone();
+	let test_dir2 = format!("{}/chain2", test_dir);
+	let wallet_proxy2 = create_wallet_proxy(&test_dir2);
+	let chain2 = wallet_proxy2.chain.clone();
+	let stopper2 = wallet_proxy2.running.clone();
+
+	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();
+
+	// Set the wallet proxy listener running
+	std::thread::spawn(move || {
+		if let Err(e) = wallet_proxy.run() {
+			error!("Wallet Proxy error: {}", e);
+		}
+	});
+
+	owner(Some(wallet1.clone()), mask1, None, |api, m| {
+		api.create_account_path(m, "a")?;
+		api.set_active_account(m, "a")?;
+		Ok(())
+	})?;
+
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		api.create_account_path(m, "b")?;
+		api.set_active_account(m, "b")?;
+		Ok(())
+	})?;
+
+	let reward = core::consensus::REWARD;
+	let cm = global::coinbase_maturity() as u64;
+	let sent = reward * 2;
+
+	// Mine some blocks
+	let bh = 10u64;
+	award_blocks_to_wallet(&chain, wallet1.clone(), mask1, bh as usize, false)?;
+
+	// Sanity check contents
+	owner(Some(wallet1.clone()), mask1, None, |api, m| {
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, bh * reward);
+		assert_eq!(info.amount_currently_spendable, (bh - cm) * reward);
+		assert_eq!(info.amount_reverted, 0);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		let (c, _) = libwallet::TxLogEntry::sum_confirmed(&txs);
+		assert_eq!(info.total, c);
+		assert_eq!(txs.len(), bh as usize);
+		Ok(())
+	})?;
+
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, 0);
+		assert_eq!(info.amount_currently_spendable, 0);
+		assert_eq!(info.amount_reverted, 0);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 0);
+		Ok(())
+	})?;
+
+	// Send some funds
+	let mut tx = None;
+	owner(Some(wallet1.clone()), mask1, None, |api, m| {
+		// send to send
+		let args = InitTxArgs {
+			src_acct_name: None,
+			amount: sent,
+			minimum_confirmations: cm,
+			max_outputs: 500,
+			num_change_outputs: 1,
+			selection_strategy_is_use_all: false,
+			..Default::default()
+		};
+		let slate = api.init_send_tx(m, args)?;
+		// output tx file
+		let send_file = format!("{}/part_tx_1.tx", test_dir);
+		PathToSlate(send_file.into()).put_tx(&slate)?;
+		api.tx_lock_outputs(m, &slate, 0)?;
+		let slate = client1.send_tx_slate_direct("wallet2", &slate)?;
+		let slate = api.finalize_tx(m, &slate)?;
+		tx = Some(slate.tx);
+
+		Ok(())
+	})?;
+	let tx = tx.unwrap();
+
+	// Check funds have been received
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, 0);
+		assert_eq!(info.amount_currently_spendable, 0);
+		assert_eq!(info.amount_reverted, 0);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+		assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived);
+		assert!(!tx.confirmed);
+		Ok(())
+	})?;
+
+	// Update parallel chain
+	assert_eq!(chain2.head_header().unwrap().height, 0);
+	for i in 0..bh {
+		let hash = chain.get_header_by_height(i + 1).unwrap().hash();
+		let block = chain.get_block(&hash).unwrap();
+		process_block(&chain2, block);
+	}
+	assert_eq!(chain2.head_header().unwrap().height, bh);
+
+	// Build 2 blocks at same height: 1 with the tx, 1 without
+	let head = chain.head_header().unwrap();
+	let block_with =
+		create_block_for_wallet(&chain, head.clone(), vec![&tx], wallet1.clone(), mask1)?;
+	let block_without = create_block_for_wallet(&chain, head, vec![], wallet1.clone(), mask1)?;
+
+	// Add block with tx to the chain
+	process_block(&chain, block_with.clone());
+	assert_eq!(chain.head_header().unwrap(), block_with.header);
+
+	// Add block without tx to the parallel chain
+	process_block(&chain2, block_without.clone());
+	assert_eq!(chain2.head_header().unwrap(), block_without.header);
+
+	let bh = bh + 1;
+
+	// Check funds have been confirmed
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, sent);
+		assert_eq!(info.amount_currently_spendable, sent);
+		assert_eq!(info.amount_reverted, 0);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+		assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived);
+		assert!(tx.confirmed);
+		assert!(tx.kernel_excess.is_some());
+		assert!(tx.reverted_after.is_none());
+		Ok(())
+	})?;
+
+	// Attach more blocks to the parallel chain, making it the longest one
+	award_block_to_wallet(&chain2, vec![], wallet1.clone(), mask1)?;
+	assert_eq!(chain2.head_header().unwrap().height, bh + 1);
+	let new_head = chain2
+		.get_block(&chain2.head_header().unwrap().hash())
+		.unwrap();
+
+	// Input blocks from parallel chain to original chain, updating it as well
+	// and effectively reverting the transaction
+	process_block(&chain, block_without.clone()); // This shouldn't update the head
+	assert_eq!(chain.head_header().unwrap(), block_with.header);
+	process_block(&chain, new_head.clone()); // But this should!
+	assert_eq!(chain.head_header().unwrap(), new_head.header);
+
+	let bh = bh + 1;
+
+	// Check funds have been reverted
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		api.scan(m, None, false)?;
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, 0);
+		assert_eq!(info.amount_currently_spendable, 0);
+		assert_eq!(info.amount_reverted, sent);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+		assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReverted);
+		assert!(!tx.confirmed);
+		assert!(tx.reverted_after.is_some());
+		Ok(())
+	})?;
+
+	stopper2.store(false, Ordering::Relaxed);
+	Ok((
+		chain, stopper, sent, bh, tx, wallet1, mask1_i, wallet2, mask2_i,
+	))
+}
+
+fn revert_reconfirm_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
+	let (chain, stopper, sent, bh, tx, wallet1, mask1_i, wallet2, mask2_i) = revert(test_dir)?;
+	let mask1 = mask1_i.as_ref();
+	let mask2 = mask2_i.as_ref();
+
+	// Include the tx into the chain again, the tx should no longer be reverted
+	award_block_to_wallet(&chain, vec![&tx], wallet1.clone(), mask1)?;
+
+	let bh = bh + 1;
+
+	// Check funds have been confirmed again
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, sent);
+		assert_eq!(info.amount_currently_spendable, sent);
+		assert_eq!(info.amount_reverted, 0);
+		// check tx log as well
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+		assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceived);
+		assert!(tx.confirmed);
+		assert!(tx.reverted_after.is_none());
+		Ok(())
+	})?;
+
+	// let logging finish
+	stopper.store(false, Ordering::Relaxed);
+	thread::sleep(Duration::from_millis(1000));
+	Ok(())
+}
+
+fn revert_cancel_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
+	let (_, stopper, sent, bh, _, _, _, wallet2, mask2_i) = revert(test_dir)?;
+	let mask2 = mask2_i.as_ref();
+
+	// Cancelling tx
+	owner(Some(wallet2.clone()), mask2, None, |api, m| {
+		// Sanity check
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, 0);
+		assert_eq!(info.amount_currently_spendable, 0);
+		assert_eq!(info.amount_reverted, sent);
+
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+
+		// Cancel
+		api.cancel_tx(m, Some(tx.id), None)?;
+
+		// Check updated summary info
+		let (refreshed, info) = api.retrieve_summary_info(m, true, 1)?;
+		assert!(refreshed);
+		assert_eq!(info.last_confirmed_height, bh);
+		assert_eq!(info.total, 0);
+		assert_eq!(info.amount_currently_spendable, 0);
+		assert_eq!(info.amount_reverted, 0);
+
+		// Check updated tx log
+		let (_, txs) = api.retrieve_txs(m, true, None, None)?;
+		assert_eq!(txs.len(), 1);
+		let tx = &txs[0];
+		assert_eq!(tx.tx_type, libwallet::TxLogEntryType::TxReceivedCancelled);
+		Ok(())
+	})?;
+
+	// let logging finish
+	stopper.store(false, Ordering::Relaxed);
+	thread::sleep(Duration::from_millis(1000));
+	Ok(())
+}
+
+#[test]
+fn tx_revert_reconfirm() {
+	let test_dir = "test_output/revert_tx";
+	setup(test_dir);
+	if let Err(e) = revert_reconfirm_impl(test_dir) {
+		panic!("Libwallet Error: {} - {}", e, e.backtrace().unwrap());
+	}
+	clean_output_dir(test_dir);
+}
+
+#[test]
+fn tx_revert_cancel() {
+	let test_dir = "test_output/revert_tx_cancel";
+	setup(test_dir);
+	if let Err(e) = revert_cancel_impl(test_dir) {
+		panic!("Libwallet Error: {} - {}", e, e.backtrace().unwrap());
+	}
+	clean_output_dir(test_dir);
+}

impls/src/test_framework/mod.rs

@@ -108,14 +108,13 @@ fn height_range_to_pmmr_indices_local(
 	}
 }
 
-/// Adds a block with a given reward to the chain and mines it
-pub fn add_block_with_reward(
+fn create_block_with_reward(
 	chain: &Chain,
+	prev: core::core::BlockHeader,
 	txs: Vec<&Transaction>,
 	reward_output: Output,
 	reward_kernel: TxKernel,
-) {
-	let prev = chain.head_header().unwrap();
+) -> core::core::Block {
 	let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
 	let mut b = core::core::Block::new(
 		&prev,
@@ -134,8 +133,50 @@ pub fn add_block_with_reward(
 		global::min_edge_bits(),
 	)
 	.unwrap();
-	chain.process_block(b, chain::Options::MINE).unwrap();
-	chain.validate(false).unwrap();
+	b
+}
+
+/// Adds a block with a given reward to the chain and mines it
+pub fn add_block_with_reward(
+	chain: &Chain,
+	txs: Vec<&Transaction>,
+	reward_output: Output,
+	reward_kernel: TxKernel,
+) {
+	let prev = chain.head_header().unwrap();
+	let block = create_block_with_reward(chain, prev, txs, reward_output, reward_kernel);
+	process_block(chain, block);
+}
+
+/// adds a reward output to a wallet, includes that reward in a block
+/// and return the block
+pub fn create_block_for_wallet<'a, L, C, K>(
+	chain: &Chain,
+	prev: core::core::BlockHeader,
+	txs: Vec<&Transaction>,
+	wallet: Arc<Mutex<Box<dyn WalletInst<'a, L, C, K> + 'a>>>,
+	keychain_mask: Option<&SecretKey>,
+) -> Result<core::core::Block, libwallet::Error>
+where
+	L: WalletLCProvider<'a, C, K>,
+	C: NodeClient + 'a,
+	K: keychain::Keychain + 'a,
+{
+	// build block fees
+	let fee_amt = txs.iter().map(|tx| tx.fee()).sum();
+	let block_fees = BlockFees {
+		fees: fee_amt,
+		key_id: None,
+		height: prev.height + 1,
+	};
+	// build coinbase (via api) and add block
+	let coinbase_tx = {
+		let mut w_lock = wallet.lock();
+		let w = w_lock.lc_provider()?.wallet_inst()?;
+		foreign::build_coinbase(&mut **w, keychain_mask, &block_fees, false)?
+	};
+	let block = create_block_with_reward(chain, prev, txs, coinbase_tx.output, coinbase_tx.kernel);
+	Ok(block)
 }
 
 /// adds a reward output to a wallet, includes that reward in a block, mines
@@ -152,24 +193,17 @@ where
 	C: NodeClient + 'a,
 	K: keychain::Keychain + 'a,
 {
-	// build block fees
 	let prev = chain.head_header().unwrap();
-	let fee_amt = txs.iter().map(|tx| tx.fee()).sum();
-	let block_fees = BlockFees {
-		fees: fee_amt,
-		key_id: None,
-		height: prev.height + 1,
-	};
-	// build coinbase (via api) and add block
-	let coinbase_tx = {
-		let mut w_lock = wallet.lock();
-		let w = w_lock.lc_provider()?.wallet_inst()?;
-		foreign::build_coinbase(&mut **w, keychain_mask, &block_fees, false)?
-	};
-	add_block_with_reward(chain, txs, coinbase_tx.output, coinbase_tx.kernel);
+	let block = create_block_for_wallet(chain, prev, txs, wallet, keychain_mask)?;
+	process_block(chain, block);
 	Ok(())
 }
 
+pub fn process_block(chain: &Chain, block: core::core::Block) {
+	chain.process_block(block, chain::Options::MINE).unwrap();
+	chain.validate(false).unwrap();
+}
+
 /// Award a blocks to a wallet directly
 pub fn award_blocks_to_wallet<'a, L, C, K>(
 	chain: &Chain,

libwallet/src/internal/tx.rs

@@ -300,8 +300,9 @@ where
 		return Err(ErrorKind::TransactionDoesntExist(tx_id_string).into());
 	}
 	let tx = tx_vec[0].clone();
-	if tx.tx_type != TxLogEntryType::TxSent && tx.tx_type != TxLogEntryType::TxReceived {
-		return Err(ErrorKind::TransactionNotCancellable(tx_id_string).into());
+	match tx.tx_type {
+		TxLogEntryType::TxSent | TxLogEntryType::TxReceived | TxLogEntryType::TxReverted => {}
+		_ => return Err(ErrorKind::TransactionNotCancellable(tx_id_string).into()),
 	}
 	if tx.confirmed {
 		return Err(ErrorKind::TransactionNotCancellable(tx_id_string).into());

libwallet/src/internal/updater.rs

@@ -15,7 +15,7 @@
 //! Utilities to check the status of all the outputs we have stored in
 //! the wallet storage and update them.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use uuid::Uuid;
 
 use crate::error::Error;
@@ -121,7 +121,8 @@ where
 				true => {
 					!tx_entry.confirmed
 						&& (tx_entry.tx_type == TxLogEntryType::TxReceived
-							|| tx_entry.tx_type == TxLogEntryType::TxSent)
+							|| tx_entry.tx_type == TxLogEntryType::TxSent
+							|| tx_entry.tx_type == TxLogEntryType::TxReverted)
 				}
 				false => true,
 			};
@@ -157,14 +158,13 @@ pub fn map_wallet_outputs<'a, T: ?Sized, C, K>(
 	keychain_mask: Option<&SecretKey>,
 	parent_key_id: &Identifier,
 	update_all: bool,
-) -> Result<HashMap<pedersen::Commitment, (Identifier, Option<u64>)>, Error>
+) -> Result<HashMap<pedersen::Commitment, (Identifier, Option<u64>, Option<u32>, bool)>, Error>
 where
 	T: WalletBackend<'a, C, K>,
 	C: NodeClient + 'a,
 	K: Keychain + 'a,
 {
-	let mut wallet_outputs: HashMap<pedersen::Commitment, (Identifier, Option<u64>)> =
-		HashMap::new();
+	let mut wallet_outputs = HashMap::new();
 	let keychain = wallet.keychain(keychain_mask)?;
 	let unspents: Vec<OutputData> = wallet
 		.iter()
@@ -174,7 +174,7 @@ where
 	let tx_entries = retrieve_txs(wallet, None, None, Some(&parent_key_id), true)?;
 
 	// Only select outputs that are actually involved in an outstanding transaction
-	let unspents: Vec<OutputData> = match update_all {
+	let unspents = match update_all {
 		false => unspents
 			.into_iter()
 			.filter(|x| match x.tx_log_entry.as_ref() {
@@ -192,7 +192,13 @@ where
 				.commit(out.value, &out.key_id, SwitchCommitmentType::Regular)
 				.unwrap(), // TODO: proper support for different switch commitment schemes
 		};
-		wallet_outputs.insert(commit, (out.key_id.clone(), out.mmr_index));
+		let val = (
+			out.key_id.clone(),
+			out.mmr_index,
+			out.tx_log_entry,
+			out.status == OutputStatus::Unspent,
+		);
+		wallet_outputs.insert(commit, val);
 	}
 	Ok(wallet_outputs)
 }
@@ -201,7 +207,7 @@ where
 pub fn cancel_tx_and_outputs<'a, T: ?Sized, C, K>(
 	wallet: &mut T,
 	keychain_mask: Option<&SecretKey>,
-	tx: TxLogEntry,
+	mut tx: TxLogEntry,
 	outputs: Vec<OutputData>,
 	parent_key_id: &Identifier,
 ) -> Result<(), Error>
@@ -214,7 +220,7 @@ where
 
 	for mut o in outputs {
 		// unlock locked outputs
-		if o.status == OutputStatus::Unconfirmed {
+		if o.status == OutputStatus::Unconfirmed || o.status == OutputStatus::Reverted {
 			batch.delete(&o.key_id, &o.mmr_index)?;
 		}
 		if o.status == OutputStatus::Locked {
@@ -222,12 +228,12 @@ where
 			batch.save(o)?;
 		}
 	}
-	let mut tx = tx;
-	if tx.tx_type == TxLogEntryType::TxSent {
-		tx.tx_type = TxLogEntryType::TxSentCancelled;
-	}
-	if tx.tx_type == TxLogEntryType::TxReceived {
-		tx.tx_type = TxLogEntryType::TxReceivedCancelled;
+	match tx.tx_type {
+		TxLogEntryType::TxSent => tx.tx_type = TxLogEntryType::TxSentCancelled,
+		TxLogEntryType::TxReceived | TxLogEntryType::TxReverted => {
+			tx.tx_type = TxLogEntryType::TxReceivedCancelled
+		}
+		_ => {}
 	}
 	batch.save_tx_log_entry(tx, parent_key_id)?;
 	batch.commit()?;
@@ -238,8 +244,9 @@ where
 pub fn apply_api_outputs<'a, T: ?Sized, C, K>(
 	wallet: &mut T,
 	keychain_mask: Option<&SecretKey>,
-	wallet_outputs: &HashMap<pedersen::Commitment, (Identifier, Option<u64>)>,
+	wallet_outputs: &HashMap<pedersen::Commitment, (Identifier, Option<u64>, Option<u32>, bool)>,
 	api_outputs: &HashMap<pedersen::Commitment, (String, u64, u64)>,
+	reverted_kernels: HashSet<u32>,
 	height: u64,
 	parent_key_id: &Identifier,
 ) -> Result<(), Error>
@@ -264,7 +271,7 @@ where
 			return Ok(());
 		}
 		let mut batch = wallet.batch(keychain_mask)?;
-		for (commit, (id, mmr_index)) in wallet_outputs.iter() {
+		for (commit, (id, mmr_index, _, _)) in wallet_outputs.iter() {
 			if let Ok(mut output) = batch.get(id, mmr_index) {
 				match api_outputs.get(&commit) {
 					Some(o) => {
@@ -296,12 +303,19 @@ where
 						// also mark the transaction in which this output is involved as confirmed
 						// note that one involved input/output confirmation SHOULD be enough
 						// to reliably confirm the tx
-						if !output.is_coinbase && output.status == OutputStatus::Unconfirmed {
+						if !output.is_coinbase
+							&& (output.status == OutputStatus::Unconfirmed
+								|| output.status == OutputStatus::Reverted)
+						{
 							let tx = batch.tx_log_iter().find(|t| {
 								Some(t.id) == output.tx_log_entry
 									&& t.parent_key_id == *parent_key_id
 							});
 							if let Some(mut t) = tx {
+								if t.tx_type == TxLogEntryType::TxReverted {
+									t.tx_type = TxLogEntryType::TxReceived;
+									t.reverted_after = None;
+								}
 								t.update_confirmation_ts();
 								t.confirmed = true;
 								batch.save_tx_log_entry(t, &parent_key_id)?;
@@ -310,11 +324,35 @@ where
 						output.height = o.1;
 						output.mark_unspent();
 					}
-					None => output.mark_spent(),
-				};
+					None => {
+						if !output.is_coinbase
+							&& output
+								.tx_log_entry
+								.map(|i| reverted_kernels.contains(&i))
+								.unwrap_or(false)
+						{
+							output.mark_reverted();
+						} else {
+							output.mark_spent();
+						}
+					}
+				}
 				batch.save(output)?;
 			}
 		}
+
+		for mut tx in batch.tx_log_iter() {
+			if reverted_kernels.contains(&tx.id) && tx.parent_key_id == *parent_key_id {
+				tx.tx_type = TxLogEntryType::TxReverted;
+				tx.reverted_after = tx.confirmation_ts.clone().and_then(|t| {
+					let now = chrono::Utc::now();
+					(now - t).to_std().ok()
+				});
+				tx.confirmed = false;
+				batch.save_tx_log_entry(tx, &parent_key_id)?;
+			}
+		}
+
 		{
 			batch.save_last_confirmed_height(parent_key_id, height)?;
 		}
@@ -349,11 +387,17 @@ where
 		.w2n_client()
 		.get_outputs_from_node(wallet_output_keys)?;
 
+	// For any disappeared output, check the on-chain status of the corresponding transaction kernel
+	// If it is no longer present, the transaction was reverted due to a re-org
+	let reverted_kernels =
+		find_reverted_kernels(wallet, &wallet_outputs, &api_outputs, parent_key_id)?;
+
 	apply_api_outputs(
 		wallet,
 		keychain_mask,
 		&wallet_outputs,
 		&api_outputs,
+		reverted_kernels,
 		height,
 		parent_key_id,
 	)?;
@@ -361,6 +405,53 @@ where
 	Ok(())
 }
 
+fn find_reverted_kernels<'a, T: ?Sized, C, K>(
+	wallet: &mut T,
+	wallet_outputs: &HashMap<pedersen::Commitment, (Identifier, Option<u64>, Option<u32>, bool)>,
+	api_outputs: &HashMap<pedersen::Commitment, (String, u64, u64)>,
+	parent_key_id: &Identifier,
+) -> Result<HashSet<u32>, Error>
+where
+	T: WalletBackend<'a, C, K>,
+	C: NodeClient + 'a,
+	K: Keychain + 'a,
+{
+	let mut client = wallet.w2n_client().clone();
+	let mut ids = HashSet::new();
+
+	// Get transaction IDs for outputs that are no longer unspent
+	for (commit, (_, _, tx_id, was_unspent)) in wallet_outputs {
+		if let Some(tx_id) = *tx_id {
+			if *was_unspent && !api_outputs.contains_key(commit) {
+				ids.insert(tx_id);
+			}
+		}
+	}
+
+	// Get corresponding kernels
+	let kernels = wallet
+		.tx_log_iter()
+		.filter(|t| {
+			ids.contains(&t.id)
+				&& t.parent_key_id == *parent_key_id
+				&& t.tx_type == TxLogEntryType::TxReceived
+		})
+		.filter_map(|t| {
+			t.kernel_excess
+				.map(|e| (t.id, e, t.kernel_lookup_min_height))
+		});
+
+	// Check each of the kernels on-chain
+	let mut reverted = HashSet::new();
+	for (id, excess, min_height) in kernels {
+		if client.get_kernel(&excess, min_height, None)?.is_none() {
+			reverted.insert(id);
+		}
+	}
+
+	Ok(reverted)
+}
+
 fn clean_old_unconfirmed<'a, T: ?Sized, C, K>(
 	wallet: &mut T,
 	keychain_mask: Option<&SecretKey>,
@@ -414,6 +505,7 @@ where
 	let mut awaiting_finalization_total = 0;
 	let mut unconfirmed_total = 0;
 	let mut locked_total = 0;
+	let mut reverted_total = 0;
 
 	for out in outputs {
 		match out.status {
@@ -440,6 +532,7 @@ where
 			OutputStatus::Locked => {
 				locked_total += out.value;
 			}
+			OutputStatus::Reverted => reverted_total += out.value,
 			OutputStatus::Spent => {}
 		}
 	}
@@ -453,6 +546,7 @@ where
 		amount_immature: immature_total,
 		amount_locked: locked_total,
 		amount_currently_spendable: unspent_total,
+		amount_reverted: reverted_total,
 	})
 }
 

libwallet/src/types.rs

@@ -36,6 +36,7 @@ use serde;
 use serde_json;
 use std::collections::HashMap;
 use std::fmt;
+use std::time::Duration;
 use uuid::Uuid;
 
 /// Combined trait to allow dynamic wallet dispatch
@@ -483,16 +484,26 @@ impl OutputData {
 
 	/// Marks this output as unspent if it was previously unconfirmed
 	pub fn mark_unspent(&mut self) {
-		if let OutputStatus::Unconfirmed = self.status {
-			self.status = OutputStatus::Unspent
-		};
+		match self.status {
+			OutputStatus::Unconfirmed | OutputStatus::Reverted => {
+				self.status = OutputStatus::Unspent
+			}
+			_ => {}
+		}
 	}
 
 	/// Mark an output as spent
 	pub fn mark_spent(&mut self) {
 		match self.status {
-			OutputStatus::Unspent => self.status = OutputStatus::Spent,
-			OutputStatus::Locked => self.status = OutputStatus::Spent,
+			OutputStatus::Unspent | OutputStatus::Locked => self.status = OutputStatus::Spent,
+			_ => (),
+		}
+	}
+
+	/// Mark an output as reverted
+	pub fn mark_reverted(&mut self) {
+		match self.status {
+			OutputStatus::Unspent => self.status = OutputStatus::Reverted,
 			_ => (),
 		}
 	}
@@ -511,6 +522,8 @@ pub enum OutputStatus {
 	Locked,
 	/// Spent
 	Spent,
+	/// Reverted
+	Reverted,
 }
 
 impl fmt::Display for OutputStatus {
@@ -520,6 +533,7 @@ impl fmt::Display for OutputStatus {
 			OutputStatus::Unspent => write!(f, "Unspent"),
 			OutputStatus::Locked => write!(f, "Locked"),
 			OutputStatus::Spent => write!(f, "Spent"),
+			OutputStatus::Reverted => write!(f, "Reverted"),
 		}
 	}
 }
@@ -707,6 +721,9 @@ pub struct WalletInfo {
 	/// amount locked via previous transactions
 	#[serde(with = "secp_ser::string_or_u64")]
 	pub amount_locked: u64,
+	/// amount previously confirmed, now reverted
+	#[serde(with = "secp_ser::string_or_u64")]
+	pub amount_reverted: u64,
 }
 
 /// Types of transactions that can be contained within a TXLog entry
@@ -722,6 +739,8 @@ pub enum TxLogEntryType {
 	TxReceivedCancelled,
 	/// Sent transaction that was rolled back by user
 	TxSentCancelled,
+	/// Received transaction that was reverted on-chain
+	TxReverted,
 }
 
 impl fmt::Display for TxLogEntryType {
@@ -732,6 +751,7 @@ impl fmt::Display for TxLogEntryType {
 			TxLogEntryType::TxSent => write!(f, "Sent Tx"),
 			TxLogEntryType::TxReceivedCancelled => write!(f, "Received Tx\n- Cancelled"),
 			TxLogEntryType::TxSentCancelled => write!(f, "Sent Tx\n- Cancelled"),
+			TxLogEntryType::TxReverted => write!(f, "Received Tx\n- Reverted"),
 		}
 	}
 }
@@ -791,6 +811,9 @@ pub struct TxLogEntry {
 	/// Additional info needed to stored payment proof
 	#[serde(default)]
 	pub payment_proof: Option<StoredProofInfo>,
+	/// Track the time it took for a transaction to get reverted
+	#[serde(with = "option_duration_as_secs", default)]
+	pub reverted_after: Option<Duration>,
 }
 
 impl ser::Writeable for TxLogEntry {
@@ -828,6 +851,7 @@ impl TxLogEntry {
 			kernel_excess: None,
 			kernel_lookup_min_height: None,
 			payment_proof: None,
+			reverted_after: None,
 		}
 	}
 
@@ -969,3 +993,83 @@ impl ser::Readable for WalletInitStatus {
 		serde_json::from_slice(&data[..]).map_err(|_| ser::Error::CorruptedData)
 	}
 }
+
+/// Serializes an Option<Duration> to and from a string
+pub mod option_duration_as_secs {
+	use serde::de::Error;
+	use serde::{Deserialize, Deserializer, Serializer};
+	use std::time::Duration;
+
+	///
+	pub fn serialize<S>(dur: &Option<Duration>, serializer: S) -> Result<S::Ok, S::Error>
+	where
+		S: Serializer,
+	{
+		match dur {
+			Some(dur) => serializer.serialize_str(&format!("{}", dur.as_secs())),
+			None => serializer.serialize_none(),
+		}
+	}
+
+	///
+	pub fn deserialize<'de, D>(deserializer: D) -> Result<Option<Duration>, D::Error>
+	where
+		D: Deserializer<'de>,
+	{
+		match Option::<String>::deserialize(deserializer)? {
+			Some(s) => {
+				let secs = s
+					.parse::<u64>()
+					.map_err(|err| Error::custom(err.to_string()))?;
+				Ok(Some(Duration::from_secs(secs)))
+			}
+			None => Ok(None),
+		}
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use serde_json::Value;
+
+	#[derive(Serialize, Deserialize, PartialEq, Clone, Debug)]
+	struct TestSer {
+		#[serde(with = "option_duration_as_secs", default)]
+		dur: Option<Duration>,
+	}
+
+	#[test]
+	fn duration_serde() {
+		let some = TestSer {
+			dur: Some(Duration::from_secs(100)),
+		};
+		let val = serde_json::to_value(some.clone()).unwrap();
+		if let Value::Object(o) = &val {
+			if let Value::String(s) = o.get("dur").unwrap() {
+				assert_eq!(s, "100");
+			} else {
+				panic!("Invalid type");
+			}
+		} else {
+			panic!("Invalid type")
+		}
+		assert_eq!(some, serde_json::from_value(val).unwrap());
+
+		let none = TestSer { dur: None };
+		let val = serde_json::to_value(none.clone()).unwrap();
+		if let Value::Object(o) = &val {
+			if let Value::Null = o.get("dur").unwrap() {
+				// ok
+			} else {
+				panic!("Invalid type");
+			}
+		} else {
+			panic!("Invalid type")
+		}
+		assert_eq!(none, serde_json::from_value(val).unwrap());
+
+		let none2 = serde_json::from_str::<TestSer>("{}").unwrap();
+		assert_eq!(none, none2);
+	}
+}