Get rid of HistoricTransaction::into_transaction

primitives/transaction/src/historic_transaction.rs

@@ -10,13 +10,10 @@ use nimiq_hash::{Blake2bHash, Blake2bHasher, Hash, Hasher};
 use nimiq_hash_derive::SerializeContent;
 use nimiq_keys::Address;
 use nimiq_mmr::hash::Hash as MMRHash;
-use nimiq_primitives::{coin::Coin, networks::NetworkId, policy::Policy};
+use nimiq_primitives::{coin::Coin, networks::NetworkId};
 use nimiq_serde::{Deserialize, Serialize};
 
-use crate::{
-    inherent::Inherent, EquivocationLocator, ExecutedTransaction,
-    Transaction as BlockchainTransaction,
-};
+use crate::{inherent::Inherent, EquivocationLocator, ExecutedTransaction};
 
 /// The raw transaction hash is a type wrapper.
 /// This corresponds to the hash of the transaction without the execution result.
@@ -294,29 +291,6 @@ impl HistoricTransaction {
             _ => self.executed_tx_hash().hash::<Blake2bHash>().into(),
         }
     }
-
-    /// Tries to convert an historic transaction into a regular transaction. This will work for all
-    /// historic transactions that wrap over regular transactions and reward inherents.
-    pub fn into_transaction(self) -> Result<ExecutedTransaction, IntoTransactionError> {
-        match self.data {
-            HistoricTransactionData::Basic(tx) => Ok(tx),
-            HistoricTransactionData::Reward(ev) => {
-                Ok(ExecutedTransaction::Ok(BlockchainTransaction::new_basic(
-                    Policy::COINBASE_ADDRESS,
-                    ev.reward_address,
-                    ev.value,
-                    Coin::ZERO,
-                    self.block_number,
-                    self.network_id,
-                )))
-            }
-            HistoricTransactionData::Penalize(_)
-            | HistoricTransactionData::Jail(_)
-            | HistoricTransactionData::Equivocation(_) => {
-                Err(IntoTransactionError::NoBasicTransactionMapping)
-            }
-        }
-    }
 }
 
 impl MMRHash<Blake2bHash> for HistoricTransaction {

rpc-interface/src/types.rs

@@ -273,7 +273,7 @@ impl Block {
                                     tx,
                                     block_number,
                                     micro_block.header.timestamp,
-                                    head_height,
+                                    Some(head_height),
                                 )
                             })
                             .collect(),
@@ -526,7 +526,7 @@ impl ExecutedTransaction {
         transaction: nimiq_transaction::ExecutedTransaction,
         block_number: u32,
         timestamp: u64,
-        head_height: u32,
+        head_height: Option<u32>,
     ) -> Self {
         // We obtain an internal executed transaction
         // We need to grab the internal transaction and map it to the RPC transaction structure
@@ -542,6 +542,47 @@ impl ExecutedTransaction {
             },
         }
     }
+    pub fn from_reward_event(
+        ev: RewardEvent,
+        hash: Blake2bHash,
+        network: NetworkId,
+        block_number: u32,
+        timestamp: u64,
+        cur_block_height: Option<u32>,
+    ) -> ExecutedTransaction {
+        ExecutedTransaction {
+            transaction: Transaction::from_reward_event(
+                ev,
+                hash,
+                network,
+                block_number,
+                timestamp,
+                cur_block_height,
+            ),
+            execution_result: true,
+        }
+    }
+    pub fn try_from_historic_transaction(
+        tx: HistoricTransaction,
+        cur_block_height: Option<u32>,
+    ) -> Option<ExecutedTransaction> {
+        Some(match tx.data {
+            HistoricTransactionData::Basic(inner) => {
+                Self::from_blockchain(inner, tx.block_number, tx.block_time, cur_block_height)
+            }
+            HistoricTransactionData::Reward(ref ev) => Self::from_reward_event(
+                ev.clone(),
+                tx.tx_hash().into(),
+                tx.network_id,
+                tx.block_number,
+                tx.block_time,
+                cur_block_height,
+            ),
+            HistoricTransactionData::Penalize(_) => return None,
+            HistoricTransactionData::Jail(_) => return None,
+            HistoricTransactionData::Equivocation(_) => return None,
+        })
+    }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
@@ -582,13 +623,13 @@ impl Transaction {
         transaction: nimiq_transaction::Transaction,
         block_number: u32,
         timestamp: u64,
-        head_height: u32,
+        head_height: Option<u32>,
     ) -> Self {
         Transaction::from(
             transaction,
             Some(block_number),
             Some(timestamp),
-            Some(head_height),
+            head_height,
         )
     }
 
@@ -621,6 +662,35 @@ impl Transaction {
             network_id: transaction.network_id as u8,
         }
     }
+
+    pub fn from_reward_event(
+        ev: RewardEvent,
+        hash: Blake2bHash,
+        network: NetworkId,
+        block_number: u32,
+        timestamp: u64,
+        cur_block_height: Option<u32>,
+    ) -> Transaction {
+        Transaction {
+            hash,
+            block_number: Some(block_number),
+            timestamp: Some(timestamp),
+            confirmations: cur_block_height.map(|h| h - block_number),
+            size: ev.serialized_size(), // TODO: changed
+            from: Policy::COINBASE_ADDRESS,
+            from_type: 0,
+            to: ev.reward_address.clone(),
+            to_type: 0,
+            value: ev.value,
+            fee: Coin::ZERO,
+            sender_data: vec![],
+            recipient_data: vec![],
+            flags: 0,
+            validity_start_height: block_number,
+            proof: vec![],
+            network_id: network as u8,
+        }
+    }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]

rpc-server/src/dispatchers/blockchain.rs

@@ -156,36 +156,26 @@ impl BlockchainInterface for BlockchainDispatcher {
     ) -> RPCResult<ExecutedTransaction, (), Self::Error> {
         if let BlockchainReadProxy::Full(blockchain) = self.blockchain.read() {
             // Get all the historic transactions that correspond to this hash.
-            let mut historic_tx_vec = blockchain.history_store.get_hist_tx_by_hash(&hash, None);
+            let mut hist_txs = blockchain.history_store.get_hist_tx_by_hash(&hash, None);
 
             // Unpack the transaction or raise an error.
-            let historic_tx = match historic_tx_vec.len() {
-                0 => {
-                    return Err(Error::TransactionNotFound(hash));
-                }
-                1 => historic_tx_vec.pop().unwrap(),
-                _ => {
-                    return Err(Error::MultipleTransactionsFound(hash));
-                }
-            };
+            if hist_txs.len() > 1 {
+                return Err(Error::MultipleTransactionsFound(hash));
+            }
+            let hist_tx = hist_txs
+                .pop()
+                .ok_or_else(|| Error::TransactionNotFound(hash.clone()))?;
 
             // Convert the historic transaction into a regular transaction. This will also convert
             // reward inherents.
-            let block_number = historic_tx.block_number;
-            let timestamp = historic_tx.block_time;
-
-            return match historic_tx.into_transaction() {
-                Ok(tx) => Ok(ExecutedTransaction::from_blockchain(
-                    tx,
-                    block_number,
-                    timestamp,
-                    blockchain.block_number(),
-                )
-                .into()),
-                Err(_) => Err(Error::TransactionNotFound(hash)),
-            };
+            Ok(ExecutedTransaction::try_from_historic_transaction(
+                hist_tx,
+                Some(blockchain.block_number()),
+            )
+            .ok_or_else(|| Error::TransactionNotFound(hash.clone()))?
+            .into())
         } else {
-            return Err(Error::NotSupportedForLightBlockchain);
+            Err(Error::NotSupportedForLightBlockchain)
         }
     }
 
@@ -195,30 +185,21 @@ impl BlockchainInterface for BlockchainDispatcher {
     ) -> RPCResult<Vec<ExecutedTransaction>, (), Self::Error> {
         if let BlockchainReadProxy::Full(blockchain) = self.blockchain.read() {
             // Get all the historic transactions that correspond to this block.
-            let historic_tx_vec = blockchain
+            let hist_txs = blockchain
                 .history_store
                 .get_block_transactions(block_number, None);
 
-            // Get the timestamp of the block from one of the historic transactions. This complicated
-            // setup is because we might not have any transactions.
-            let timestamp = historic_tx_vec.first().map(|x| x.block_time).unwrap_or(0);
-
-            // Convert the historic transactions into regular transactions. This will also convert
-            // reward inherents.
-            let mut transactions = vec![];
-
-            for hist_tx in historic_tx_vec {
-                if let Ok(tx) = hist_tx.into_transaction() {
-                    transactions.push(ExecutedTransaction::from_blockchain(
-                        tx,
-                        block_number,
-                        timestamp,
-                        blockchain.block_number(),
-                    ));
-                }
-            }
-
-            Ok(transactions.into())
+            let cur_block_height = blockchain.block_number();
+            let result: Vec<_> = hist_txs
+                .into_iter()
+                .filter_map(|hist_tx| {
+                    ExecutedTransaction::try_from_historic_transaction(
+                        hist_tx,
+                        Some(cur_block_height),
+                    )
+                })
+                .collect();
+            Ok(result.into())
         } else {
             Err(Error::NotSupportedForLightBlockchain)
         }
@@ -265,25 +246,15 @@ impl BlockchainInterface for BlockchainDispatcher {
             // Search all micro blocks of the batch to find the transactions.
             let mut transactions = vec![];
 
+            let cur_block_height = blockchain.block_number();
             for i in first_block..=last_block {
                 let hist_txs = blockchain.history_store.get_block_transactions(i, None);
-
-                // Get the timestamp of the block from one of the historic transactions. This complicated
-                // setup is because we might not have any transactions.
-                let timestamp = hist_txs.first().map(|x| x.block_time).unwrap_or(0);
-
-                // Convert the historic transactions into regular transactions. This will also convert
-                // reward inherents.
-                for hist_tx in hist_txs {
-                    if let Ok(tx) = hist_tx.into_transaction() {
-                        transactions.push(ExecutedTransaction::from_blockchain(
-                            tx,
-                            i,
-                            timestamp,
-                            blockchain.block_number(),
-                        ));
-                    }
-                }
+                transactions.extend(hist_txs.into_iter().filter_map(|hist_tx| {
+                    ExecutedTransaction::try_from_historic_transaction(
+                        hist_tx,
+                        Some(cur_block_height),
+                    )
+                }));
             }
 
             Ok(transactions.into())
@@ -367,32 +338,25 @@ impl BlockchainInterface for BlockchainDispatcher {
 
             for hash in tx_hashes {
                 // Get all the historic transactions that correspond to this hash.
-                let mut historic_tx_vec = blockchain.history_store.get_hist_tx_by_hash(&hash, None);
+                let mut hist_txs = blockchain.history_store.get_hist_tx_by_hash(&hash, None);
 
                 // Unpack the transaction or raise an error.
-                let historic_tx = match historic_tx_vec.len() {
-                    0 => {
-                        return Err(Error::TransactionNotFound(hash));
-                    }
-                    1 => historic_tx_vec.pop().unwrap(),
-                    _ => {
-                        return Err(Error::MultipleTransactionsFound(hash));
-                    }
-                };
+                if hist_txs.len() > 1 {
+                    return Err(Error::MultipleTransactionsFound(hash));
+                }
+                let hist_tx = hist_txs
+                    .pop()
+                    .ok_or_else(|| Error::TransactionNotFound(hash.clone()))?;
 
                 // Convert the historic transaction into a regular transaction. This will also convert
                 // reward inherents.
-                let block_number = historic_tx.block_number;
-                let timestamp = historic_tx.block_time;
-
-                if let Ok(tx) = historic_tx.into_transaction() {
-                    txs.push(ExecutedTransaction::from_blockchain(
-                        tx,
-                        block_number,
-                        timestamp,
-                        blockchain.block_number(),
-                    ));
-                }
+                txs.push(
+                    ExecutedTransaction::try_from_historic_transaction(
+                        hist_tx,
+                        Some(blockchain.block_number()),
+                    )
+                    .ok_or_else(|| Error::TransactionNotFound(hash.clone()))?,
+                )
             }
 
             Ok(txs.into())