Make fee and timestamp in TransactionDetails Options

.github/workflows/cont_integration.yml

@@ -78,6 +78,7 @@ jobs:
     name: Test ${{ matrix.blockchain.name }}
     runs-on: ubuntu-16.04
     strategy:
+      fail-fast: false
       matrix:
         blockchain:
           - name: electrum

src/blockchain/compact_filters/mod.rs

@@ -71,7 +71,7 @@ use super::{Blockchain, Capability, ConfigurableBlockchain, Progress};
 use crate::database::{BatchDatabase, BatchOperations, DatabaseUtils};
 use crate::error::Error;
 use crate::types::{KeychainKind, LocalUtxo, TransactionDetails};
-use crate::FeeRate;
+use crate::{ConfirmationTime, FeeRate};
 
 use peer::*;
 use store::*;
@@ -146,7 +146,7 @@ impl CompactFiltersBlockchain {
         database: &mut D,
         tx: &Transaction,
         height: Option<u32>,
-        timestamp: u64,
+        timestamp: Option<u64>,
         internal_max_deriv: &mut Option<u32>,
         external_max_deriv: &mut Option<u32>,
     ) -> Result<(), Error> {
@@ -206,9 +206,8 @@ impl CompactFiltersBlockchain {
                 transaction: Some(tx.clone()),
                 received: incoming,
                 sent: outgoing,
-                height,
-                timestamp,
-                fees: inputs_sum.saturating_sub(outputs_sum),
+                confirmation_time: ConfirmationTime::new(height, timestamp),
+                fee: Some(inputs_sum.saturating_sub(outputs_sum)),
             };
 
             info!("Saving tx {}", tx.txid);
@@ -364,8 +363,8 @@ impl Blockchain for CompactFiltersBlockchain {
         );
         let mut updates = database.begin_batch();
         for details in database.iter_txs(false)? {
-            match details.height {
-                Some(height) if (height as usize) < last_synced_block => continue,
+            match details.confirmation_time {
+                Some(c) if (c.height as usize) < last_synced_block => continue,
                 _ => updates.del_tx(&details.txid, false)?,
             };
         }
@@ -387,7 +386,7 @@ impl Blockchain for CompactFiltersBlockchain {
                     database,
                     tx,
                     Some(height as u32),
-                    0,
+                    None,
                     &mut internal_max_deriv,
                     &mut external_max_deriv,
                 )?;
@@ -398,7 +397,7 @@ impl Blockchain for CompactFiltersBlockchain {
                 database,
                 tx,
                 None,
-                0,
+                None,
                 &mut internal_max_deriv,
                 &mut external_max_deriv,
             )?;

src/blockchain/rpc.rs

@@ -33,7 +33,7 @@ use crate::blockchain::{Blockchain, Capability, ConfigurableBlockchain, Progress
 use crate::database::{BatchDatabase, DatabaseUtils};
 use crate::descriptor::{get_checksum, IntoWalletDescriptor};
 use crate::wallet::utils::SecpCtx;
-use crate::{Error, FeeRate, KeychainKind, LocalUtxo, TransactionDetails};
+use crate::{ConfirmationTime, Error, FeeRate, KeychainKind, LocalUtxo, TransactionDetails};
 use bitcoincore_rpc::json::{
     GetAddressInfoResultLabel, ImportMultiOptions, ImportMultiRequest,
     ImportMultiRequestScriptPubkey, ImportMultiRescanSince,
@@ -189,13 +189,15 @@ impl Blockchain for RpcBlockchain {
             let txid = tx_result.info.txid;
             list_txs_ids.insert(txid);
             if let Some(mut known_tx) = known_txs.get_mut(&txid) {
-                if tx_result.info.blockheight != known_tx.height {
+                let confirmation_time =
+                    ConfirmationTime::new(tx_result.info.blockheight, tx_result.info.blocktime);
+                if confirmation_time != known_tx.confirmation_time {
                     // reorg may change tx height
                     debug!(
-                        "updating tx({}) height to: {:?}",
-                        txid, tx_result.info.blockheight
+                        "updating tx({}) confirmation time to: {:?}",
+                        txid, confirmation_time
                     );
-                    known_tx.height = tx_result.info.blockheight;
+                    known_tx.confirmation_time = confirmation_time;
                     db.set_tx(&known_tx)?;
                 }
             } else {
@@ -224,17 +226,17 @@ impl Blockchain for RpcBlockchain {
                 let td = TransactionDetails {
                     transaction: Some(tx),
                     txid: tx_result.info.txid,
-                    timestamp: tx_result.info.time,
+                    confirmation_time: ConfirmationTime::new(
+                        tx_result.info.blockheight,
+                        tx_result.info.blocktime,
+                    ),
                     received,
                     sent,
-                    //TODO it could happen according to the node situation/configuration that the
-                    // fee is not known [TransactionDetails:fee] should be made [Option]
-                    fees: tx_result.fee.map(|f| f.as_sat().abs() as u64).unwrap_or(0),
-                    height: tx_result.info.blockheight,
+                    fee: tx_result.fee.map(|f| f.as_sat().abs() as u64),
                 };
                 debug!(
                     "saving tx: {} tx_result.fee:{:?} td.fees:{:?}",
-                    td.txid, tx_result.fee, td.fees
+                    td.txid, tx_result.fee, td.fee
                 );
                 db.set_tx(&td)?;
             }
@@ -519,7 +521,7 @@ mod test {
         wallet.sync(noop_progress(), None).unwrap();
         assert_eq!(
             wallet.get_balance().unwrap(),
-            100_000 - 50_000 - details.fees
+            100_000 - 50_000 - details.fee.unwrap_or(0)
         );
         drop(wallet);
 

src/blockchain/utils.rs

@@ -21,7 +21,7 @@ use bitcoin::{BlockHeader, OutPoint, Script, Transaction, Txid};
 use super::*;
 use crate::database::{BatchDatabase, BatchOperations, DatabaseUtils};
 use crate::error::Error;
-use crate::types::{KeychainKind, LocalUtxo, TransactionDetails};
+use crate::types::{ConfirmationTime, KeychainKind, LocalUtxo, TransactionDetails};
 use crate::wallet::time::Instant;
 use crate::wallet::utils::ChunksIterator;
 
@@ -147,13 +147,14 @@ pub trait ElectrumLikeSync {
         // save any tx details not in db but in history_txs_id or with different height/timestamp
         for txid in history_txs_id.iter() {
             let height = txid_height.get(txid).cloned().flatten();
-            let timestamp = *new_timestamps.get(txid).unwrap_or(&0u64);
+            let timestamp = new_timestamps.get(txid).cloned();
             if let Some(tx_details) = txs_details_in_db.get(txid) {
-                // check if height matches, otherwise updates it
-                if tx_details.height != height {
+                // check if tx height matches, otherwise updates it. timestamp is not in the if clause
+                // because we are not asking headers for confirmed tx we know about
+                if tx_details.confirmation_time.as_ref().map(|c| c.height) != height {
+                    let confirmation_time = ConfirmationTime::new(height, timestamp);
                     let mut new_tx_details = tx_details.clone();
-                    new_tx_details.height = height;
-                    new_tx_details.timestamp = timestamp;
+                    new_tx_details.confirmation_time = confirmation_time;
                     batch.set_tx(&new_tx_details)?;
                 }
             } else {
@@ -238,9 +239,13 @@ pub trait ElectrumLikeSync {
         chunk_size: usize,
     ) -> Result<HashMap<Txid, u64>, Error> {
         let mut txid_timestamp = HashMap::new();
+        let txid_in_db_with_conf: HashSet<_> = txs_details_in_db
+            .values()
+            .filter_map(|details| details.confirmation_time.as_ref().map(|_| details.txid))
+            .collect();
         let needed_txid_height: HashMap<&Txid, u32> = txid_height
             .iter()
-            .filter(|(t, _)| txs_details_in_db.get(*t).is_none())
+            .filter(|(t, _)| !txid_in_db_with_conf.contains(*t))
             .filter_map(|(t, o)| o.map(|h| (t, h)))
             .collect();
         let needed_heights: HashSet<u32> = needed_txid_height.values().cloned().collect();
@@ -292,7 +297,7 @@ pub trait ElectrumLikeSync {
 fn save_transaction_details_and_utxos<D: BatchDatabase>(
     txid: &Txid,
     db: &mut D,
-    timestamp: u64,
+    timestamp: Option<u64>,
     height: Option<u32>,
     updates: &mut dyn BatchOperations,
     utxo_deps: &HashMap<OutPoint, OutPoint>,
@@ -355,9 +360,8 @@ fn save_transaction_details_and_utxos<D: BatchDatabase>(
         transaction: Some(tx),
         received: incoming,
         sent: outgoing,
-        height,
-        timestamp,
-        fees: inputs_sum.saturating_sub(outputs_sum), /* if the tx is a coinbase, fees would be negative */
+        confirmation_time: ConfirmationTime::new(height, timestamp),
+        fee: Some(inputs_sum.saturating_sub(outputs_sum)), /* if the tx is a coinbase, fees would be negative */
     };
     updates.set_tx(&tx_details)?;
 

src/database/memory.rs

@@ -473,18 +473,18 @@ macro_rules! populate_test_db {
         };
 
         let txid = tx.txid();
-        let height = tx_meta
-            .min_confirmations
-            .map(|conf| current_height.unwrap().checked_sub(conf as u32).unwrap());
+        let confirmation_time = tx_meta.min_confirmations.map(|conf| ConfirmationTime {
+            height: current_height.unwrap().checked_sub(conf as u32).unwrap(),
+            timestamp: 0,
+        });
 
         let tx_details = TransactionDetails {
             transaction: Some(tx.clone()),
             txid,
-            timestamp: 0,
-            height,
+            fee: Some(0),
             received: 0,
             sent: 0,
-            fees: 0,
+            confirmation_time,
         };
 
         db.set_tx(&tx_details).unwrap();

src/database/mod.rs

@@ -314,11 +314,13 @@ pub mod test {
         let mut tx_details = TransactionDetails {
             transaction: Some(tx),
             txid,
-            timestamp: 123456,
             received: 1337,
             sent: 420420,
-            fees: 140,
-            height: Some(1000),
+            fee: Some(140),
+            confirmation_time: Some(ConfirmationTime {
+                timestamp: 123456,
+                height: 1000,
+            }),
         };
 
         tree.set_tx(&tx_details).unwrap();