Merge pull request #1971 from mintlayer/chainstate_test_fw_and_api_srv_cleanup

Chainstate test framework and api server cleanup

Author: ImplOfAnImpl

Cargo.lock

@@ -21,6 +21,7 @@ async-trait.workspace = true
 bb8-postgres = "0.8"
 clap = { workspace = true, features = ["derive"] }
 futures = { workspace = true, default-features = false }
+itertools.workspace = true
 parity-scale-codec.workspace = true
 thiserror.workspace = true
 tokio = { workspace = true, features = ["full"] }

api-server/api-server-common/Cargo.toml

@@ -31,6 +31,7 @@ use common::{
     },
     primitives::{id::WithId, Amount, BlockHeight, CoinOrTokenId, Id, Idable},
 };
+use itertools::Itertools as _;
 use std::{
     cmp::Reverse,
     collections::{BTreeMap, BTreeSet},
@@ -61,15 +62,14 @@ struct ApiServerInMemoryStorage {
     statistics:
         BTreeMap<CoinOrTokenStatistic, BTreeMap<CoinOrTokenId, BTreeMap<BlockHeight, Amount>>>,
     orders_table: BTreeMap<OrderId, BTreeMap<BlockHeight, Order>>,
-    best_block: BlockAuxData,
     genesis_block: Arc<WithId<Genesis>>,
     number_of_coin_decimals: u8,
     storage_version: u32,
 }
 
 impl ApiServerInMemoryStorage {
     pub fn new(chain_config: &ChainConfig) -> Self {
-        let mut result = Self {
+        Self {
             block_table: BTreeMap::new(),
             block_aux_data_table: BTreeMap::new(),
             address_balance_table: BTreeMap::new(),
@@ -89,18 +89,9 @@ impl ApiServerInMemoryStorage {
             statistics: BTreeMap::new(),
             orders_table: BTreeMap::new(),
             genesis_block: chain_config.genesis_block().clone(),
-            best_block: BlockAuxData::new(
-                chain_config.genesis_block_id(),
-                0.into(),
-                chain_config.genesis_block().timestamp(),
-            ),
             number_of_coin_decimals: chain_config.coin_decimals(),
-            storage_version: super::CURRENT_STORAGE_VERSION,
-        };
-        result
-            .initialize_storage(chain_config)
-            .expect("In-memory initialization must succeed");
-        result
+            storage_version: CURRENT_STORAGE_VERSION,
+        }
     }
 
     fn is_initialized(&self) -> Result<bool, ApiServerStorageError> {
@@ -295,7 +286,18 @@ impl ApiServerInMemoryStorage {
     }
 
     fn get_best_block(&self) -> Result<BlockAuxData, ApiServerStorageError> {
-        Ok(self.best_block)
+        let result = self.main_chain_blocks_table.last_key_value().map_or_else(
+            || {
+                BlockAuxData::new(
+                    self.genesis_block.get_id().into(),
+                    0.into(),
+                    self.genesis_block.timestamp(),
+                )
+            },
+            |(_, id)| *self.block_aux_data_table.get(id).expect("must exist"),
+        );
+
+        Ok(result)
     }
 
     fn get_latest_blocktimestamps(&self) -> Result<Vec<BlockTimestamp>, ApiServerStorageError> {
@@ -331,28 +333,18 @@ impl ApiServerInMemoryStorage {
         &self,
         time_range: (BlockTimestamp, BlockTimestamp),
     ) -> Result<(BlockHeight, BlockHeight), ApiServerStorageError> {
-        let from_height = self
-            .main_chain_blocks_table
-            .iter()
-            .find_map(|(k, v)| {
-                (self.block_aux_data_table.get(v).expect("must exist").block_timestamp()
-                    >= time_range.0)
-                    .then_some(*k)
-            })
-            .unwrap_or(BlockHeight::new(0));
-
-        let to_height = self
+        let result = self
             .main_chain_blocks_table
             .iter()
-            .rev()
-            .find_map(|(k, v)| {
-                (self.block_aux_data_table.get(v).expect("must exist").block_timestamp()
-                    <= time_range.1)
-                    .then_some(*k)
+            .filter_map(|(h, id)| {
+                let ts = self.block_aux_data_table.get(id).expect("must exist").block_timestamp();
+                (ts >= time_range.0 && ts <= time_range.1).then_some(*h)
             })
-            .unwrap_or(BlockHeight::new(0));
+            .minmax()
+            .into_option()
+            .unwrap_or((BlockHeight::new(0), BlockHeight::new(0)));
 
-        Ok((from_height, to_height))
+        Ok(result)
     }
 
     fn get_delegation(
@@ -798,35 +790,13 @@ impl ApiServerInMemoryStorage {
 }
 
 impl ApiServerInMemoryStorage {
-    fn initialize_storage(
-        &mut self,
-        _chain_config: &ChainConfig,
-    ) -> Result<(), ApiServerStorageError> {
-        self.storage_version = CURRENT_STORAGE_VERSION;
-
-        Ok(())
-    }
-
     fn reinitialize_storage(
         &mut self,
         chain_config: &ChainConfig,
     ) -> Result<(), ApiServerStorageError> {
-        self.block_table.clear();
-        self.block_aux_data_table.clear();
-        self.address_balance_table.clear();
-        self.address_locked_balance_table.clear();
-        self.address_transactions_table.clear();
-        self.delegation_table.clear();
-        self.main_chain_blocks_table.clear();
-        self.pool_data_table.clear();
-        self.transaction_table.clear();
-        self.utxo_table.clear();
-        self.address_utxos.clear();
-        self.fungible_token_data.clear();
-        self.nft_token_issuances.clear();
-        self.orders_table.clear();
-
-        self.initialize_storage(chain_config)
+        let mut new_storage = Self::new(chain_config);
+        std::mem::swap(self, &mut new_storage);
+        Ok(())
     }
 
     fn del_address_balance_above_height(
@@ -971,13 +941,22 @@ impl ApiServerInMemoryStorage {
         block_height: BlockHeight,
         block: &BlockWithExtraData,
     ) -> Result<(), ApiServerStorageError> {
+        let previously_stored_height =
+            self.block_aux_data_table.get(&block_id).map(|data| data.block_height());
+
+        let aux_data = BlockAuxData::new(block_id.into(), block_height, block.block.timestamp());
         self.block_table.insert(block_id, block.clone());
-        self.block_aux_data_table.insert(
-            block_id,
-            BlockAuxData::new(block_id.into(), block_height, block.block.timestamp()),
-        );
+        self.block_aux_data_table.insert(block_id, aux_data);
         self.main_chain_blocks_table.insert(block_height, block_id);
-        self.best_block = BlockAuxData::new(block_id.into(), block_height, block.block.timestamp());
+
+        // Handle a degenerate case when the block is stored several times using different heights
+        // (to be consistent with the postgres implementation).
+        if let Some(previously_stored_height) = previously_stored_height {
+            if previously_stored_height != block_height {
+                self.main_chain_blocks_table.remove(&previously_stored_height);
+            }
+        }
+
         Ok(())
     }
 

api-server/api-server-common/src/storage/impls/in_memory/mod.rs

@@ -1134,11 +1134,19 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
         let height = Self::block_height_to_postgres_friendly(block_height);
         let timestamp = Self::block_time_to_postgres_friendly(block.block.timestamp())?;
 
+        // Note: we need to update the block height on conflict, because the block may have been
+        // added as a stale one in the past, in which case its block_height hasn't been set.
+        // Regarding the rest of the columns, updating them on conflict makes no sense from the
+        // consensus point of view, because the data, if calculated correctly, can never change,
+        // provided that the block id stays the same. However, this is a low-level db call,
+        // so it'd be better if the caller decides what data can change and what can't.
+        // Also, this way it's consistent with the in-memory implementation.
         self.tx
             .execute(
-                "INSERT INTO ml.blocks (block_id, block_height, block_timestamp, block_data) VALUES ($1, $2, $3, $4)
+                "INSERT INTO ml.blocks (block_id, block_height, block_timestamp, block_data)
+                    VALUES ($1, $2, $3, $4)
                     ON CONFLICT (block_id) DO UPDATE
-                    SET block_data = $4, block_height = $2;",
+                    SET block_height = $2, block_timestamp = $3, block_data = $4;",
                 &[&block_id.encode(), &height, &timestamp, &block.encode()],
             )
             .await
@@ -2705,6 +2713,7 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
     ) -> Result<(), ApiServerStorageError> {
         logging::log::debug!("Inserting block aux data with block_id {}", block_id);
 
+        // Note: we update the data on block id conflict, the reasons are the same as in set_mainchain_block.
         self.tx
             .execute(
                 "INSERT INTO ml.block_aux_data (block_id, aux_data) VALUES ($1, $2)

api-server/api-server-common/src/storage/impls/postgres/queries.rs

@@ -611,6 +611,11 @@ pub trait ApiServerStorageRead: Sync {
         block_id: Id<Block>,
     ) -> Result<Option<BlockAuxData>, ApiServerStorageError>;
 
+    // Note:
+    // 1) The input time range is inclusive on both ends.
+    // 2) The returned heights won't include the genesis height normally.
+    // However, if there are no blocks on the mainchain in the specified time range (except genesis),
+    // the function will return (0, 0).
     async fn get_block_range_from_time_range(
         &self,
         time_range: (BlockTimestamp, BlockTimestamp),

api-server/api-server-common/src/storage/storage_api/mod.rs

@@ -13,6 +13,7 @@ chainstate = { path = "../../chainstate" }
 chainstate-test-framework = { path = "../../chainstate/test-framework" }
 common = { path = "../../common" }
 crypto = { path = "../../crypto" }
+logging = { path = "../../logging" }
 randomness = { path = "../../randomness" }
 serialization = { path = "../../serialization" }
 test-utils = { path = "../../test-utils" }
@@ -23,10 +24,11 @@ mempool = { path = "../../mempool" }
 
 async-trait.workspace = true
 axum.workspace = true
+ctor.workspace = true
 hex.workspace = true
 libtest-mimic.workspace = true
 reqwest = "0.11"
+rstest.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 tokio = { workspace = true, features = ["full"] }
-rstest.workspace = true

api-server/stack-test-suite/Cargo.toml

@@ -83,6 +83,11 @@ use std::{
 };
 use test_utils::random::{make_seedable_rng, Rng, Seed};
 
+#[ctor::ctor]
+fn init() {
+    logging::init_logging();
+}
+
 #[tokio::test]
 async fn chain_genesis() {
     let url = "/api/v2/chain/genesis";

api-server/stack-test-suite/tests/v2/mod.rs

@@ -191,20 +191,20 @@ async fn multiple_tx_in_same_block(#[case] seed: Seed) {
                 let transaction = signed_tx2.transaction();
 
                 let expected_transaction = json!({
-                "block_id": block_id.to_hash().encode_hex::<String>(),
-                "timestamp": block.timestamp().to_string(),
-                "confirmations": BlockHeight::new(0).to_string(),
-                "version_byte": transaction.version_byte(),
-                "is_replaceable": transaction.is_replaceable(),
-                "flags": transaction.flags(),
-                "inputs": transaction.inputs().iter().zip(utxos).map(|(inp, utxo)| json!({
-                    "input": tx_input_to_json(inp, &TokenDecimals::Single(None), &chain_config),
-                    "utxo": utxo.as_ref().map(|txo| txoutput_to_json(txo, &chain_config, &TokenDecimals::Single(None))),
+                    "block_id": block_id.to_hash().encode_hex::<String>(),
+                    "timestamp": block.timestamp().to_string(),
+                    "confirmations": BlockHeight::new(0).to_string(),
+                    "version_byte": transaction.version_byte(),
+                    "is_replaceable": transaction.is_replaceable(),
+                    "flags": transaction.flags(),
+                    "inputs": transaction.inputs().iter().zip(utxos).map(|(inp, utxo)| json!({
+                        "input": tx_input_to_json(inp, &TokenDecimals::Single(None), &chain_config),
+                        "utxo": utxo.as_ref().map(|txo| txoutput_to_json(txo, &chain_config, &TokenDecimals::Single(None))),
                     })).collect::<Vec<_>>(),
-                "outputs": transaction.outputs()
-                            .iter()
-                            .map(|out| txoutput_to_json(out, &chain_config, &TokenDecimals::Single(None)))
-                            .collect::<Vec<_>>(),
+                    "outputs": transaction.outputs()
+                        .iter()
+                        .map(|out| txoutput_to_json(out, &chain_config, &TokenDecimals::Single(None)))
+                        .collect::<Vec<_>>(),
                 });
 
                 _ = tx.send((
@@ -337,20 +337,20 @@ async fn ok(#[case] seed: Seed) {
                 });
 
                 let expected_transaction = json!({
-                "block_id": block_id.to_hash().encode_hex::<String>(),
-                "timestamp": block.timestamp().to_string(),
-                "confirmations": BlockHeight::new((n_blocks - block_height) as u64).to_string(),
-                "version_byte": transaction.version_byte(),
-                "is_replaceable": transaction.is_replaceable(),
-                "flags": transaction.flags(),
-                "inputs": transaction.inputs().iter().zip(utxos).map(|(inp, utxo)| json!({
-                    "input": tx_input_to_json(inp, &TokenDecimals::Single(None), &chain_config),
-                    "utxo": utxo.as_ref().map(|txo| txoutput_to_json(txo, &chain_config, &TokenDecimals::Single(None))),
+                    "block_id": block_id.to_hash().encode_hex::<String>(),
+                    "timestamp": block.timestamp().to_string(),
+                    "confirmations": BlockHeight::new((n_blocks - block_height) as u64).to_string(),
+                    "version_byte": transaction.version_byte(),
+                    "is_replaceable": transaction.is_replaceable(),
+                    "flags": transaction.flags(),
+                    "inputs": transaction.inputs().iter().zip(utxos).map(|(inp, utxo)| json!({
+                        "input": tx_input_to_json(inp, &TokenDecimals::Single(None), &chain_config),
+                        "utxo": utxo.as_ref().map(|txo| txoutput_to_json(txo, &chain_config, &TokenDecimals::Single(None))),
                     })).collect::<Vec<_>>(),
-                "outputs": transaction.outputs()
-                            .iter()
-                            .map(|out| txoutput_to_json(out, &chain_config, &TokenDecimals::Single(None)))
-                            .collect::<Vec<_>>(),
+                    "outputs": transaction.outputs()
+                        .iter()
+                        .map(|out| txoutput_to_json(out, &chain_config, &TokenDecimals::Single(None)))
+                        .collect::<Vec<_>>(),
                 });
 
                 _ = tx.send((

api-server/stack-test-suite/tests/v2/transaction.rs

@@ -13,11 +13,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use serde_json::Value;
+
 use api_server_common::storage::storage_api::{
     block_aux_data::BlockAuxData, TransactionInfo, TxAdditionalInfo,
 };
 use api_web_server::api::json_helpers::to_tx_json_with_block_info;
-use serde_json::Value;
 
 use super::*;