Merge pull request #355 from mintlayer/fix/read_block_reward

Avoid reading the whole block to get the block reward

Author: TheQuantumPhysicist

chainstate-storage/src/internal/mod.rs

@@ -18,6 +18,7 @@ pub mod utxo_db;
 use chainstate_types::BlockIndex;
 use common::{
     chain::{
+        block::BlockReward,
         transaction::{Transaction, TxMainChainIndex, TxMainChainPosition},
         Block, GenBlock, OutPoint, OutPointSourceId,
     },
@@ -141,6 +142,7 @@ impl<B: for<'tx> traits::Transactional<'tx, Schema>> BlockchainStorageRead for S
         fn get_best_block_id(&self) -> crate::Result<Option<Id<GenBlock>>>;
         fn get_block_index(&self, id: &Id<Block>) -> crate::Result<Option<BlockIndex>>;
         fn get_block(&self, id: Id<Block>) -> crate::Result<Option<Block>>;
+        fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>>;
 
         fn get_mainchain_tx_index(
             &self,
@@ -226,6 +228,22 @@ impl<Tx: for<'a> traits::GetMapRef<'a, Schema>> BlockchainStorageRead for StoreT
         self.read::<DBBlock, _, _>(id.as_ref())
     }
 
+    fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>> {
+        match self.0.get::<DBBlock, _>().get(block_index.block_id().as_ref()) {
+            Err(e) => Err(e.into()),
+            Ok(None) => Ok(None),
+            Ok(Some(block)) => {
+                let header_size = block_index.block_header().encoded_size();
+                let begin = header_size;
+                let encoded_block_reward_begin =
+                    block.get(begin..).expect("Block reward outside of block range");
+                let block_reward = BlockReward::decode(&mut &*encoded_block_reward_begin)
+                    .expect("Invalid block reward encoding in DB");
+                Ok(Some(block_reward))
+            }
+        }
+    }
+
     fn get_mainchain_tx_index(
         &self,
         tx_id: &OutPointSourceId,

chainstate-storage/src/lib.rs

@@ -16,6 +16,7 @@
 //! Application-level interface for the persistent blockchain storage.
 
 use chainstate_types::BlockIndex;
+use common::chain::block::BlockReward;
 use common::chain::transaction::{Transaction, TxMainChainIndex, TxMainChainPosition};
 use common::chain::OutPointSourceId;
 use common::chain::{Block, GenBlock};
@@ -49,6 +50,8 @@ pub trait BlockchainStorageRead: UtxosStorageRead {
 
     fn get_block_index(&self, block_id: &Id<Block>) -> crate::Result<Option<BlockIndex>>;
 
+    fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>>;
+
     /// Get block by its hash
     fn get_block(&self, id: Id<Block>) -> crate::Result<Option<Block>>;
 

chainstate-storage/src/mock.rs

@@ -18,6 +18,7 @@
 use chainstate_types::BlockIndex;
 use common::{
     chain::{
+        block::BlockReward,
         transaction::{OutPointSourceId, Transaction, TxMainChainIndex, TxMainChainPosition},
         Block, GenBlock, OutPoint,
     },
@@ -37,6 +38,7 @@ mockall::mock! {
         fn get_best_block_id(&self) -> crate::Result<Option<Id<GenBlock>>>;
         fn get_block_index(&self, id: &Id<Block>) -> crate::Result<Option<BlockIndex>>;
         fn get_block(&self, id: Id<Block>) -> crate::Result<Option<Block>>;
+        fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>>;
 
         fn get_mainchain_tx_index(
             &self,
@@ -113,6 +115,7 @@ mockall::mock! {
         fn get_best_block_id(&self) -> crate::Result<Option<Id<GenBlock>>>;
         fn get_block_index(&self, id: &Id<Block>) -> crate::Result<Option<BlockIndex>>;
         fn get_block(&self, id: Id<Block>) -> crate::Result<Option<Block>>;
+        fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>>;
 
         fn get_mainchain_tx_index(
             &self,
@@ -151,6 +154,7 @@ mockall::mock! {
         fn get_best_block_id(&self) -> crate::Result<Option<Id<GenBlock>>>;
         fn get_block(&self, id: Id<Block>) -> crate::Result<Option<Block>>;
         fn get_block_index(&self, id: &Id<Block>) -> crate::Result<Option<BlockIndex>>;
+        fn get_block_reward(&self, block_index: &BlockIndex) -> crate::Result<Option<BlockReward>>;
 
         fn get_mainchain_tx_index(
             &self,

chainstate/src/detail/ban_score.rs

@@ -92,6 +92,7 @@ impl BanScore for ConnectTransactionError {
             ConnectTransactionError::InvariantErrorTransactionCouldNotBeLoaded(_) => 100,
             // Even though this is an invariant error, it stems from a block reward that doesn't exist
             ConnectTransactionError::InvariantErrorHeaderCouldNotBeLoaded(_) => 100,
+            ConnectTransactionError::InvariantErrorBlockIndexCouldNotBeLoaded(_) => 100,
             ConnectTransactionError::InvariantErrorBlockCouldNotBeLoaded(_) => 100,
             ConnectTransactionError::FailedToAddAllFeesOfBlock(_) => 100,
             ConnectTransactionError::RewardAdditionError(_) => 100,

chainstate/src/detail/chainstateref.rs

@@ -19,7 +19,9 @@ use chainstate_storage::{BlockchainStorageRead, BlockchainStorageWrite, Transact
 use chainstate_types::{get_skip_height, BlockIndex, GenBlockIndex, PropertyQueryError};
 use common::{
     chain::{
-        block::{calculate_tx_merkle_root, calculate_witness_merkle_root, BlockHeader},
+        block::{
+            calculate_tx_merkle_root, calculate_witness_merkle_root, BlockHeader, BlockReward,
+        },
         Block, ChainConfig, GenBlock, GenBlockId, OutPointSourceId,
     },
     primitives::{BlockDistance, BlockHeight, Id, Idable},
@@ -53,19 +55,28 @@ impl<'a, S: BlockchainStorageRead, O: OrphanBlocks> BlockIndexHandle for Chainst
     ) -> Result<Option<BlockIndex>, PropertyQueryError> {
         self.get_block_index(block_id)
     }
+
     fn get_gen_block_index(
         &self,
         block_id: &Id<GenBlock>,
     ) -> Result<Option<GenBlockIndex>, PropertyQueryError> {
         self.get_gen_block_index(block_id)
     }
+
     fn get_ancestor(
         &self,
         block_index: &BlockIndex,
         ancestor_height: BlockHeight,
     ) -> Result<GenBlockIndex, PropertyQueryError> {
         self.get_ancestor(&GenBlockIndex::Block(block_index.clone()), ancestor_height)
     }
+
+    fn get_block_reward(
+        &self,
+        block_index: &BlockIndex,
+    ) -> Result<Option<BlockReward>, PropertyQueryError> {
+        self.get_block_reward(block_index)
+    }
 }
 
 impl<'a, S: BlockchainStorageRead, O: OrphanBlocks> TransactionIndexHandle
@@ -308,6 +319,13 @@ impl<'a, S: BlockchainStorageRead, O: OrphanBlocks> ChainstateRef<'a, S, O> {
         Ok(self.get_block_index(&id)?.map(|block_index| block_index.into_block_header()))
     }
 
+    pub fn get_block_reward(
+        &self,
+        block_index: &BlockIndex,
+    ) -> Result<Option<BlockReward>, PropertyQueryError> {
+        Ok(self.db_tx.get_block_reward(block_index)?)
+    }
+
     pub fn get_block_height_in_main_chain(
         &self,
         id: &Id<GenBlock>,

chainstate/src/detail/mod.rs

@@ -33,7 +33,11 @@ use itertools::Itertools;
 use chainstate_storage::{BlockchainStorage, Transactional};
 use chainstate_types::{BlockIndex, GenBlockIndex, PropertyQueryError};
 use common::{
-    chain::{block::BlockHeader, config::ChainConfig, Block, GenBlock},
+    chain::{
+        block::{BlockHeader, BlockReward},
+        config::ChainConfig,
+        Block, GenBlock,
+    },
     primitives::{BlockDistance, BlockHeight, Id, Idable},
 };
 use logging::log;
@@ -293,6 +297,14 @@ impl<S: BlockchainStorage> Chainstate<S> {
         self.make_db_tx_ro().get_best_block_id()
     }
 
+    #[allow(dead_code)]
+    pub fn get_block_reward(
+        &self,
+        block_index: &BlockIndex,
+    ) -> Result<Option<BlockReward>, PropertyQueryError> {
+        self.make_db_tx_ro().get_block_reward(block_index)
+    }
+
     #[allow(dead_code)]
     pub fn get_header_from_height(
         &self,

chainstate/src/detail/tests/processing_tests.rs

@@ -26,7 +26,7 @@ use common::{
     primitives::Compact,
     Uint256,
 };
-use consensus::{ConsensusPoWError, ConsensusVerificationError};
+use consensus::{BlockIndexHandle, ConsensusPoWError, ConsensusVerificationError};
 use crypto::key::{KeyKind, PrivateKey};
 
 use crate::{
@@ -639,6 +639,88 @@ fn pow(#[case] seed: Seed) {
     tf.process_block(valid_block.clone(), BlockSource::Local).unwrap();
 }
 
+#[rstest]
+#[trace]
+#[case(Seed::from_entropy())]
+fn read_block_reward_from_storage(#[case] seed: Seed) {
+    let mut rng = make_seedable_rng(seed);
+    let ignore_consensus = BlockHeight::new(0);
+    let pow_consensus = BlockHeight::new(1);
+    let difficulty =
+        Uint256([0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFF, 0x0FFFFFFFFFFFFFFF]);
+
+    let upgrades = vec![
+        (
+            ignore_consensus,
+            UpgradeVersion::ConsensusUpgrade(ConsensusUpgrade::IgnoreConsensus),
+        ),
+        (
+            pow_consensus,
+            UpgradeVersion::ConsensusUpgrade(ConsensusUpgrade::PoW {
+                initial_difficulty: difficulty.into(),
+            }),
+        ),
+    ];
+
+    let net_upgrades = NetUpgrades::initialize(upgrades).expect("valid netupgrades");
+    // Internally this calls Consensus::new, which processes the genesis block
+    // This should succeed because TestChainConfig by default uses create_mainnet_genesis to
+    // create the genesis_block, and this function creates a genesis block with
+    // ConsensusData::None, which agrees with the net_upgrades we defined above.
+    let chain_config = ConfigBuilder::test_chain().net_upgrades(net_upgrades).build();
+    let mut tf = TestFramework::builder().with_chain_config(chain_config).build();
+
+    let block_reward_output_count = rng.gen::<usize>() % 20;
+    let expected_block_reward = (0..block_reward_output_count)
+        .map(|_| {
+            let amount = Amount::from_atoms(rng.gen::<u128>() % 50);
+            let pub_key = PrivateKey::new(KeyKind::RistrettoSchnorr).1;
+            TxOutput::new(
+                OutputValue::Coin(amount),
+                OutputPurpose::Transfer(Destination::PublicKey(pub_key)),
+            )
+        })
+        .collect::<Vec<_>>();
+
+    // We generate a PoW block, then use its reward to test the storage of block rewards
+    let block = {
+        let mut random_invalid_block = tf
+            .make_block_builder()
+            .with_reward(expected_block_reward.clone())
+            .add_test_transaction(&mut rng)
+            .build();
+        make_invalid_pow_block(&mut random_invalid_block, u128::MAX, difficulty.into())
+            .expect("generate invalid block");
+
+        let mut valid_block = random_invalid_block;
+        let bits = difficulty.into();
+        assert!(consensus::pow::mine(&mut valid_block, u128::MAX, bits)
+            .expect("Unexpected conversion error"));
+        valid_block
+    };
+    tf.process_block(block, BlockSource::Local).unwrap();
+
+    let block_index = tf.chainstate.get_best_block_index().unwrap().unwrap();
+    let block_index = match block_index {
+        GenBlockIndex::Block(bi) => bi,
+        GenBlockIndex::Genesis(_) => unreachable!(),
+    };
+
+    {
+        let block_reward = tf.chainstate.get_block_reward(&block_index).unwrap().unwrap();
+
+        assert_eq!(block_reward.outputs(), expected_block_reward);
+    }
+
+    {
+        let block_index_handle = &tf.chainstate.make_db_tx_ro() as &dyn BlockIndexHandle;
+
+        let block_reward = block_index_handle.get_block_reward(&block_index).unwrap().unwrap();
+
+        assert_eq!(block_reward.outputs(), expected_block_reward);
+    }
+}
+
 #[test]
 fn blocks_from_the_future() {
     common::concurrency::model(|| {

chainstate/src/detail/transaction_verifier/error.rs

@@ -74,6 +74,8 @@ pub enum ConnectTransactionError {
     InvariantErrorTransactionCouldNotBeLoaded(TxMainChainPosition),
     #[error("Transaction index for header found but header not found")]
     InvariantErrorHeaderCouldNotBeLoaded(Id<Block>),
+    #[error("Unable to find block index")]
+    InvariantErrorBlockIndexCouldNotBeLoaded(Id<Block>),
     #[error("Unable to find block")]
     InvariantErrorBlockCouldNotBeLoaded(Id<Block>),
     #[error("Addition of all fees in block `{0}` failed")]

chainstate/src/detail/transaction_verifier/mod.rs

@@ -644,14 +644,19 @@ impl<'a, S: BlockchainStorageRead> TransactionVerifier<'a, S> {
                 .to_vec()),
             // TODO: Getting the whole block just for reward outputs isn't optimal. See the
             // https://github.com/mintlayer/mintlayer-core/issues/344 issue for details.
-            GenBlockId::Block(id) => Ok(self
-                .db_tx
-                .get_block(id)?
-                .ok_or(ConnectTransactionError::InvariantErrorBlockCouldNotBeLoaded(id))?
-                .block_reward_transactable()
-                .outputs()
-                .unwrap_or(&[])
-                .to_vec()),
+            GenBlockId::Block(id) => {
+                let block_index = self
+                    .db_tx
+                    .get_block_index(&id)?
+                    .ok_or(ConnectTransactionError::InvariantErrorBlockIndexCouldNotBeLoaded(id))?;
+                let reward = self
+                    .db_tx
+                    .get_block_reward(&block_index)?
+                    .ok_or(ConnectTransactionError::InvariantErrorBlockCouldNotBeLoaded(id))?
+                    .outputs()
+                    .to_vec();
+                Ok(reward)
+            }
         }
     }
 

consensus/src/validator/block_index_handle.rs

@@ -15,7 +15,7 @@
 
 use chainstate_types::{BlockIndex, GenBlockIndex, PropertyQueryError};
 use common::{
-    chain::{Block, GenBlock},
+    chain::{block::BlockReward, Block, GenBlock},
     primitives::{BlockHeight, Id},
 };
 
@@ -39,4 +39,10 @@ pub trait BlockIndexHandle {
         block_index: &BlockIndex,
         ancestor_height: BlockHeight,
     ) -> Result<GenBlockIndex, PropertyQueryError>;
+
+    /// Returns the block reward of the given block
+    fn get_block_reward(
+        &self,
+        block_index: &BlockIndex,
+    ) -> Result<Option<BlockReward>, PropertyQueryError>;
 }