fix: tms validation correctly updating

applications/minotari_app_grpc/proto/block.proto

@@ -121,7 +121,6 @@ message NewBlockHeaderTemplate {
     bytes total_kernel_offset = 4;
     // Proof of work metadata
     ProofOfWork pow = 5;
-//    uint64 target_difficulty = 6;
     // Sum of script offsets for all kernels in this block.
     bytes total_script_offset = 7;
 }

applications/minotari_app_grpc/proto/wallet.proto

@@ -227,6 +227,8 @@ enum TransactionStatus {
     TRANSACTION_STATUS_COINBASE_UNCONFIRMED = 12;
     // This is Coinbase transaction that is detected from chain
     TRANSACTION_STATUS_COINBASE_CONFIRMED = 13;
+    // This is Coinbase transaction that is not currently detected as mined
+    TRANSACTION_STATUS_COINBASE_NOT_IN_BLOCK_CHAIN = 14;
 }
 
 message GetCompletedTransactionsRequest { }

applications/minotari_app_grpc/src/conversions/transaction.rs

@@ -104,6 +104,7 @@ impl From<TransactionStatus> for grpc::TransactionStatus {
             OneSidedConfirmed => grpc::TransactionStatus::OneSidedConfirmed,
             CoinbaseUnconfirmed => grpc::TransactionStatus::CoinbaseUnconfirmed,
             CoinbaseConfirmed => grpc::TransactionStatus::CoinbaseConfirmed,
+            CoinbaseNotInBlockChain => grpc::TransactionStatus::CoinbaseNotInBlockChain,
             Queued => grpc::TransactionStatus::Queued,
         }
     }

base_layer/common_types/src/transaction.rs

@@ -38,10 +38,13 @@ pub enum TransactionStatus {
     OneSidedConfirmed = 9,
     /// This transaction is still being queued for initial sending
     Queued = 10,
-    /// This transaction import status is used when a one-sided transaction has been scanned but is unconfirmed
+    /// This transaction import status is used when a coinbase transaction has been scanned but is unconfirmed
     CoinbaseUnconfirmed = 11,
-    /// This transaction import status is used when a one-sided transaction has been scanned and confirmed
+    /// This transaction import status is used when a coinbase transaction has been scanned and confirmed
     CoinbaseConfirmed = 12,
+    /// This transaction import status is used when a coinbase transaction has been scanned but the outputs are not
+    /// currently confirmed on the blockchain via the output manager
+    CoinbaseNotInBlockChain = 13,
 }
 
 impl TransactionStatus {
@@ -55,7 +58,9 @@ impl TransactionStatus {
     pub fn is_coinbase(&self) -> bool {
         matches!(
             self,
-            TransactionStatus::CoinbaseUnconfirmed | TransactionStatus::CoinbaseConfirmed
+            TransactionStatus::CoinbaseUnconfirmed |
+                TransactionStatus::CoinbaseConfirmed |
+                TransactionStatus::CoinbaseNotInBlockChain
         )
     }
 
@@ -81,9 +86,9 @@ impl TransactionStatus {
             TransactionStatus::Imported |
             TransactionStatus::OneSidedUnconfirmed |
             TransactionStatus::OneSidedConfirmed => TransactionStatus::OneSidedConfirmed,
-            TransactionStatus::CoinbaseConfirmed | TransactionStatus::CoinbaseUnconfirmed => {
-                TransactionStatus::CoinbaseConfirmed
-            },
+            TransactionStatus::CoinbaseNotInBlockChain |
+            TransactionStatus::CoinbaseConfirmed |
+            TransactionStatus::CoinbaseUnconfirmed => TransactionStatus::CoinbaseConfirmed,
         }
     }
 
@@ -100,9 +105,9 @@ impl TransactionStatus {
             TransactionStatus::Imported |
             TransactionStatus::OneSidedUnconfirmed |
             TransactionStatus::OneSidedConfirmed => TransactionStatus::OneSidedUnconfirmed,
-            TransactionStatus::CoinbaseConfirmed | TransactionStatus::CoinbaseUnconfirmed => {
-                TransactionStatus::CoinbaseUnconfirmed
-            },
+            TransactionStatus::CoinbaseConfirmed |
+            TransactionStatus::CoinbaseUnconfirmed |
+            TransactionStatus::CoinbaseNotInBlockChain => TransactionStatus::CoinbaseUnconfirmed,
         }
     }
 }
@@ -131,6 +136,7 @@ impl TryFrom<i32> for TransactionStatus {
             10 => Ok(TransactionStatus::Queued),
             11 => Ok(TransactionStatus::CoinbaseUnconfirmed),
             12 => Ok(TransactionStatus::CoinbaseConfirmed),
+            13 => Ok(TransactionStatus::CoinbaseNotInBlockChain),
             code => Err(TransactionConversionError { code }),
         }
     }
@@ -152,6 +158,7 @@ impl Display for TransactionStatus {
             TransactionStatus::OneSidedConfirmed => write!(f, "One-Sided Confirmed"),
             TransactionStatus::CoinbaseUnconfirmed => write!(f, "Coinbase Unconfirmed"),
             TransactionStatus::CoinbaseConfirmed => write!(f, "Coinbase Confirmed"),
+            TransactionStatus::CoinbaseNotInBlockChain => write!(f, "Coinbase not mined"),
             TransactionStatus::Queued => write!(f, "Queued"),
         }
     }
@@ -165,9 +172,9 @@ pub enum ImportStatus {
     OneSidedUnconfirmed,
     /// This transaction import status is used when a one-sided transaction has been scanned and confirmed
     OneSidedConfirmed,
-    /// This transaction import status is used when a one-sided transaction has been scanned but is unconfirmed
+    /// This transaction import status is used when a coinbasetransaction has been scanned but is unconfirmed
     CoinbaseUnconfirmed,
-    /// This transaction import status is used when a one-sided transaction has been scanned and confirmed
+    /// This transaction import status is used when a coinbase transaction has been scanned and confirmed
     CoinbaseConfirmed,
 }
 

base_layer/core/src/base_node/rpc/service.rs

@@ -313,7 +313,7 @@ impl<B: BlockchainBackend + 'static> BaseNodeWalletService for BaseNodeWalletRpc
                 location: response.location,
                 block_hash: response.block_hash,
                 confirmations: response.confirmations,
-                block_height: response.height_of_longest_chain - response.confirmations,
+                block_height: response.height_of_longest_chain.saturating_sub(response.confirmations),
                 mined_timestamp: response.mined_timestamp,
             });
         }

base_layer/wallet/src/transaction_service/protocols/transaction_validation_protocol.rs

@@ -285,7 +285,10 @@ where
                 .map_err(TransactionServiceError::ProtobufConversionError)?;
             let sig = response.signature;
             if let Some(unconfirmed_tx) = batch_signatures.get(&sig) {
-                if response.location == TxLocation::Mined && response.block_hash.is_some() {
+                if response.location == TxLocation::Mined &&
+                    response.block_hash.is_some() &&
+                    response.mined_timestamp.is_some()
+                {
                     mined.push((
                         (*unconfirmed_tx).clone(),
                         response.block_height,
@@ -296,10 +299,8 @@ where
                 } else {
                     warn!(
                         target: LOG_TARGET,
-                        "Marking transaction {} as unmined and confirmed '{}' with block '{}' (Operation ID: {})",
+                        "Transaction {} is unmined (Operation ID: {})",
                         &unconfirmed_tx.tx_id,
-                        response.confirmations >= self.config.num_confirmations_required,
-                        response.block_hash.is_some(),
                         self.operation_id,
                     );
                     unmined.push((*unconfirmed_tx).clone());

base_layer/wallet/src/transaction_service/service.rs

@@ -2530,7 +2530,7 @@ where
             JoinHandle<Result<OperationId, TransactionServiceProtocolError<OperationId>>>,
         >,
     ) -> Result<OperationId, TransactionServiceError> {
-        self.resources.db.mark_all_transactions_as_unvalidated()?;
+        self.resources.db.mark_all_non_coinbases_transactions_as_unvalidated()?;
         self.start_transaction_validation_protocol(join_handles).await
     }
 

base_layer/wallet/src/transaction_service/storage/database.rs

@@ -136,7 +136,7 @@ pub trait TransactionBackend: Send + Sync + Clone {
     /// Clears the mined block and height of a transaction
     fn set_transaction_as_unmined(&self, tx_id: TxId) -> Result<(), TransactionStorageError>;
     /// Reset optional 'mined height' and 'mined in block' fields to nothing
-    fn mark_all_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError>;
+    fn mark_all_non_coinbases_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError>;
     /// Light weight method to retrieve pertinent transaction sender info for all pending inbound transactions
     fn get_pending_inbound_transaction_sender_info(
         &self,
@@ -147,6 +147,10 @@ pub trait TransactionBackend: Send + Sync + Clone {
         &self,
         height: u64,
     ) -> Result<Vec<CompletedTransaction>, TransactionStorageError>;
+    fn fetch_unmined_coinbase_transactions_from_height(
+        &self,
+        height: u64,
+    ) -> Result<Vec<CompletedTransaction>, TransactionStorageError>;
 }
 
 #[derive(Clone, PartialEq)]
@@ -429,6 +433,14 @@ where T: TransactionBackend + 'static
         Ok(t)
     }
 
+    pub fn get_unmined_coinbase_transactions(
+        &self,
+        height: u64,
+    ) -> Result<Vec<CompletedTransaction>, TransactionStorageError> {
+        let t = self.db.fetch_unmined_coinbase_transactions_from_height(height)?;
+        Ok(t)
+    }
+
     pub fn fetch_last_mined_transaction(&self) -> Result<Option<CompletedTransaction>, TransactionStorageError> {
         self.db.fetch_last_mined_transaction()
     }
@@ -683,8 +695,8 @@ where T: TransactionBackend + 'static
         self.db.set_transaction_as_unmined(tx_id)
     }
 
-    pub fn mark_all_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError> {
-        self.db.mark_all_transactions_as_unvalidated()
+    pub fn mark_all_non_coinbases_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError> {
+        self.db.mark_all_non_coinbases_transactions_as_unvalidated()
     }
 
     pub fn set_transaction_mined_height(

base_layer/wallet/src/transaction_service/storage/sqlite_db.rs

@@ -918,18 +918,23 @@ impl TransactionBackend for TransactionServiceSqliteDatabase {
         Ok(result)
     }
 
-    fn mark_all_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError> {
+    // Exclude coinbases as they are validated from the OMS service, and we use these fields to know which tx to
+    // extract, thus we should not wipe it out. Coinbases can also not be mined in a different height so the data will
+    // never be wrong.
+    fn mark_all_non_coinbases_transactions_as_unvalidated(&self) -> Result<(), TransactionStorageError> {
         let start = Instant::now();
         let mut conn = self.database_connection.get_pooled_connection()?;
         let acquire_lock = start.elapsed();
         let result = diesel::update(completed_transactions::table)
+            .filter(completed_transactions::status.ne(TransactionStatus::CoinbaseNotInBlockChain as i32))
+            .filter(completed_transactions::status.ne(TransactionStatus::CoinbaseUnconfirmed as i32))
+            .filter(completed_transactions::status.ne(TransactionStatus::CoinbaseConfirmed as i32))
             .set((
                 completed_transactions::cancelled.eq::<Option<i32>>(None),
                 completed_transactions::mined_height.eq::<Option<i64>>(None),
                 completed_transactions::mined_in_block.eq::<Option<Vec<u8>>>(None),
             ))
             .execute(&mut conn)?;
-
         trace!(target: LOG_TARGET, "rows updated: {:?}", result);
         if start.elapsed().as_millis() > 0 {
             trace!(
@@ -1035,6 +1040,27 @@ impl TransactionBackend for TransactionServiceSqliteDatabase {
         Ok(coinbases)
     }
 
+    fn fetch_unmined_coinbase_transactions_from_height(
+        &self,
+        height: u64,
+    ) -> Result<Vec<CompletedTransaction>, TransactionStorageError> {
+        let mut conn = self.database_connection.get_pooled_connection()?;
+        let cipher = acquire_read_lock!(self.cipher);
+
+        let coinbases = CompletedTransactionSql::index_by_status_and_cancelled_from_block_height(
+            TransactionStatus::CoinbaseNotInBlockChain,
+            false,
+            height as i64,
+            &mut conn,
+        )?
+        .into_iter()
+        .map(|ct: CompletedTransactionSql| {
+            CompletedTransaction::try_from(ct, &cipher).map_err(TransactionStorageError::from)
+        })
+        .collect::<Result<Vec<CompletedTransaction>, TransactionStorageError>>()?;
+        Ok(coinbases)
+    }
+
     fn fetch_confirmed_detected_transactions_from_height(
         &self,
         height: u64,
@@ -1849,37 +1875,37 @@ impl CompletedTransactionSql {
     }
 
     pub fn set_as_unmined(tx_id: TxId, conn: &mut SqliteConnection) -> Result<(), TransactionStorageError> {
-        // This query uses two sub-queries to retrieve existing values in the table
+        let (current_status, current_mined_height) = *completed_transactions::table
+            .filter(completed_transactions::tx_id.eq(tx_id.as_u64() as i64))
+            .select((completed_transactions::status, completed_transactions::mined_height))
+            .load::<(i32, Option<i64>)>(conn)?
+            .first()
+            .ok_or(TransactionStorageError::DieselError(DieselError::NotFound))?;
+        let current_status = TransactionStatus::try_from(current_status)
+            .map_err(|_| TransactionStorageError::UnexpectedResult("Unknown status".to_string()))?;
         diesel::update(completed_transactions::table.filter(completed_transactions::tx_id.eq(tx_id.as_u64() as i64)))
             .set(UpdateCompletedTransactionSql {
-                status: {
-                    if let Some(status) = completed_transactions::table
-                        .filter(completed_transactions::tx_id.eq(tx_id.as_u64() as i64))
-                        .select(completed_transactions::status)
-                        .load::<i32>(conn)?
-                        .first()
-                    {
-                        if *status == TransactionStatus::OneSidedConfirmed as i32 ||
-                            *status == TransactionStatus::OneSidedUnconfirmed as i32
-                        {
-                            Some(TransactionStatus::OneSidedUnconfirmed as i32)
-                        } else if *status == TransactionStatus::CoinbaseUnconfirmed as i32 ||
-                            *status == TransactionStatus::CoinbaseConfirmed as i32
-                        {
-                            Some(TransactionStatus::CoinbaseUnconfirmed as i32)
-                        } else if *status == TransactionStatus::Imported as i32 {
-                            Some(TransactionStatus::Imported as i32)
-                        } else if *status == TransactionStatus::Broadcast as i32 {
-                            Some(TransactionStatus::Broadcast as i32)
-                        } else {
-                            Some(TransactionStatus::Completed as i32)
-                        }
+                status: match current_status {
+                    TransactionStatus::OneSidedConfirmed | TransactionStatus::OneSidedUnconfirmed => {
+                        Some(TransactionStatus::OneSidedUnconfirmed as i32)
+                    },
+                    TransactionStatus::CoinbaseUnconfirmed |
+                    TransactionStatus::CoinbaseConfirmed |
+                    TransactionStatus::CoinbaseNotInBlockChain => {
+                        Some(TransactionStatus::CoinbaseNotInBlockChain as i32)
+                    },
+                    TransactionStatus::Imported => Some(TransactionStatus::Imported as i32),
+                    TransactionStatus::Broadcast => Some(TransactionStatus::Broadcast as i32),
+                    _ => Some(TransactionStatus::Completed as i32),
+                },
+                mined_in_block: Some(None),
+                mined_height: {
+                    if current_status.is_coinbase() {
+                        Some(current_mined_height)
                     } else {
-                        return Err(TransactionStorageError::DieselError(DieselError::NotFound));
+                        Some(None)
                     }
                 },
-                mined_in_block: Some(None),
-                mined_height: Some(None),
                 confirmations: Some(None),
                 // Turns out it should not be cancelled
                 cancelled: Some(None),
@@ -2109,6 +2135,7 @@ impl UnconfirmedTransactionInfoSql {
                     .and(completed_transactions::status.ne(TransactionStatus::OneSidedConfirmed as i32))
                     .and(completed_transactions::status.ne(TransactionStatus::CoinbaseUnconfirmed as i32))
                     .and(completed_transactions::status.ne(TransactionStatus::CoinbaseConfirmed as i32))
+                    .and(completed_transactions::status.ne(TransactionStatus::CoinbaseNotInBlockChain as i32))
                     // Filter out any transaction without a kernel signature
                     .and(completed_transactions::transaction_signature_nonce.ne(vec![0u8; 32]))
                     .and(completed_transactions::transaction_signature_key.ne(vec![0u8; 32]))