Merge branch 'yuji/save-conversion-state' (#2190)

* origin/yuji/save-conversion-state:
  fix to restore conversion state when epoch 0
  add changelog
  fix rollback test
  save conversion state only for restarting

.changelog/unreleased/improvements/2189-save-conversion-state.md

@@ -0,0 +1,2 @@
+- Save MASP conversion state to the state storage instead of the diffs
+  ([\#2189](https://github.com/anoma/namada/issues/2189))

apps/src/lib/bench_utils.rs

@@ -804,7 +804,7 @@ impl Default for BenchShieldedCtx {
         }
 
         crate::wallet::save(&chain_ctx.wallet).unwrap();
-        namada::ledger::storage::update_allowed_conversions(
+        namada::core::ledger::masp_conversions::update_allowed_conversions(
             &mut shell.wl_storage,
         )
         .unwrap();

apps/src/lib/client/rpc.rs

@@ -37,7 +37,6 @@ use namada::ledger::parameters::{storage as param_storage, EpochDuration};
 use namada::ledger::pos::types::{CommissionPair, Slash};
 use namada::ledger::pos::PosParams;
 use namada::ledger::queries::RPC;
-use namada::ledger::storage::ConversionState;
 use namada::proof_of_stake::types::{ValidatorState, WeightedValidator};
 use namada::types::address::{Address, InternalAddress, MASP};
 use namada::types::hash::Hash;
@@ -2343,20 +2342,12 @@ pub async fn query_conversions<'a>(
         .wallet()
         .await
         .get_addresses_with_vp_type(AddressVpType::Token);
-    let masp_addr = MASP;
-    let key_prefix: Key = masp_addr.to_db_key().into();
-    let state_key = key_prefix
-        .push(&(token::CONVERSION_KEY_PREFIX.to_owned()))
-        .unwrap();
-    let conv_state: ConversionState =
-        query_storage_value(context.client(), &state_key)
-            .await
-            .expect("Conversions should be defined");
+    let conversions = rpc::query_conversions(context.client())
+        .await
+        .expect("Conversions should be defined");
     // Track whether any non-sentinel conversions are found
     let mut conversions_found = false;
-    for ((addr, _), epoch, conv, _) in conv_state.assets.values() {
-        let amt: masp_primitives::transaction::components::I128Sum =
-            conv.clone().into();
+    for (addr, epoch, amt) in conversions.values() {
         // If the user has specified any targets, then meet them
         // If we have a sentinel conversion, then skip printing
         if matches!(&target_token, Some(target) if target != addr)
@@ -2378,7 +2369,7 @@ pub async fn query_conversions<'a>(
         for (asset_type, val) in amt.components() {
             // Look up the address and epoch of asset to facilitate pretty
             // printing
-            let ((addr, _), epoch, _, _) = &conv_state.assets[asset_type];
+            let (addr, epoch, _) = &conversions[asset_type];
             // Now print out this component of the conversion
             display!(
                 context.io(),

apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -2,6 +2,7 @@
 
 use data_encoding::HEXUPPER;
 use namada::core::ledger::inflation;
+use namada::core::ledger::masp_conversions::update_allowed_conversions;
 use namada::core::ledger::pgf::ADDRESS as pgf_address;
 use namada::ledger::events::EventType;
 use namada::ledger::gas::{GasMetering, TxGasMeter};
@@ -92,9 +93,7 @@ where
             namada_proof_of_stake::read_pos_params(&self.wl_storage)?;
 
         if new_epoch {
-            namada::ledger::storage::update_allowed_conversions(
-                &mut self.wl_storage,
-            )?;
+            update_allowed_conversions(&mut self.wl_storage)?;
 
             execute_governance_proposals(self, &mut response)?;
 

apps/src/lib/node/ledger/shell/init_chain.rs

@@ -293,14 +293,6 @@ where
                     .insert(alias, address.clone());
             }
         }
-        let key_prefix: Key = address::MASP.to_db_key().into();
-        // Save the current conversion state
-        let state_key = key_prefix
-            .push(&(token::CONVERSION_KEY_PREFIX.to_owned()))
-            .unwrap();
-        let conv_bytes =
-            self.wl_storage.storage.conversion_state.serialize_to_vec();
-        self.wl_storage.write_bytes(&state_key, conv_bytes).unwrap();
     }
 
     /// Init genesis token balances

apps/src/lib/node/ledger/shell/mod.rs

@@ -1550,12 +1550,11 @@ mod test_utils {
     use std::path::PathBuf;
 
     use data_encoding::HEXUPPER;
+    use namada::core::ledger::masp_conversions::update_allowed_conversions;
     use namada::core::ledger::storage::EPOCH_SWITCH_BLOCKS_DELAY;
     use namada::ledger::parameters::{EpochDuration, Parameters};
     use namada::ledger::storage::mockdb::MockDB;
-    use namada::ledger::storage::{
-        update_allowed_conversions, LastBlock, Sha256Hasher,
-    };
+    use namada::ledger::storage::{LastBlock, Sha256Hasher};
     use namada::ledger::storage_api::StorageWrite;
     use namada::proof_of_stake::parameters::PosParams;
     use namada::proof_of_stake::validator_consensus_key_handle;

apps/src/lib/node/ledger/storage/mod.rs

@@ -54,13 +54,12 @@ mod tests {
     use std::collections::HashMap;
 
     use itertools::Itertools;
+    use namada::core::ledger::masp_conversions::update_allowed_conversions;
     use namada::ledger::gas::STORAGE_ACCESS_GAS_PER_BYTE;
     use namada::ledger::ibc::storage::ibc_key;
     use namada::ledger::parameters::{EpochDuration, Parameters};
     use namada::ledger::storage::write_log::WriteLog;
-    use namada::ledger::storage::{
-        types, update_allowed_conversions, StoreType, WlStorage,
-    };
+    use namada::ledger::storage::{types, StoreType, WlStorage};
     use namada::ledger::storage_api::{self, StorageWrite};
     use namada::types::chain::ChainId;
     use namada::types::hash::Hash;

apps/src/lib/node/ledger/storage/rocksdb.rs

@@ -17,6 +17,7 @@
 //!     - `tx_queue`
 //!     - `next_epoch_min_start_height`
 //!     - `next_epoch_min_start_time`
+//!   - `conversion_state`: MASP conversion state
 //! - `subspace`: accounts sub-spaces
 //!   - `{address}/{dyn}`: any byte data associated with accounts
 //! - `diffs`: diffs in account subspaces' key-vals
@@ -47,6 +48,7 @@ use borsh::BorshDeserialize;
 use borsh_ext::BorshSerializeExt;
 use data_encoding::HEXLOWER;
 use itertools::Either;
+use namada::core::ledger::masp_conversions::ConversionState;
 use namada::core::types::ethereum_structs;
 use namada::ledger::storage::merkle_tree::{
     base_tree_key_prefix, subtree_key_prefix,
@@ -483,6 +485,19 @@ impl RocksDB {
             // Tendermint.
         }
 
+        // Revert conversion state if the epoch had been changed
+        if last_block.pred_epochs.get_epoch(previous_height)
+            != Some(last_block.epoch)
+        {
+            let previous_key = "pred/conversion_state".to_string();
+            let previous_value = self
+                .0
+                .get_cf(state_cf, previous_key.as_bytes())
+                .map_err(|e| Error::DBError(e.to_string()))?
+                .ok_or(Error::UnknownKey { key: previous_key })?;
+            batch.put_cf(state_cf, "conversion_state", previous_value);
+        }
+
         // Delete block results for the last block
         let block_cf = self.get_column_family(BLOCK_CF)?;
         tracing::info!("Removing last block results");
@@ -658,6 +673,17 @@ impl DB for RocksDB {
                 return Ok(None);
             }
         };
+        let conversion_state: ConversionState = match self
+            .0
+            .get_cf(state_cf, "conversion_state")
+            .map_err(|e| Error::DBError(e.into_string()))?
+        {
+            Some(bytes) => types::decode(bytes).map_err(Error::CodingError)?,
+            None => {
+                tracing::error!("Couldn't load conversion state from the DB");
+                return Ok(None);
+            }
+        };
         let tx_queue: TxQueue = match self
             .0
             .get_cf(state_cf, "tx_queue")
@@ -820,6 +846,7 @@ impl DB for RocksDB {
                 epoch,
                 pred_epochs,
                 results,
+                conversion_state,
                 next_epoch_min_start_height,
                 next_epoch_min_start_time,
                 update_epoch_blocks_delay,
@@ -854,6 +881,7 @@ impl DB for RocksDB {
             update_epoch_blocks_delay,
             address_gen,
             results,
+            conversion_state,
             tx_queue,
             ethereum_height,
             eth_events_queue,
@@ -914,6 +942,27 @@ impl DB for RocksDB {
             types::encode(&update_epoch_blocks_delay),
         );
 
+        // Save the conversion state when the epoch is updated
+        if is_full_commit {
+            if let Some(current_value) = self
+                .0
+                .get_cf(state_cf, "conversion_state")
+                .map_err(|e| Error::DBError(e.into_string()))?
+            {
+                // Write the predecessor value for rollback
+                batch.0.put_cf(
+                    state_cf,
+                    "pred/conversion_state",
+                    current_value,
+                );
+            }
+            batch.0.put_cf(
+                state_cf,
+                "conversion_state",
+                types::encode(conversion_state),
+            );
+        }
+
         // Tx queue
         if let Some(pred_tx_queue) = self
             .0
@@ -1655,7 +1704,9 @@ mod imp {
 #[cfg(test)]
 mod test {
     use namada::ledger::storage::{MerkleTree, Sha256Hasher};
-    use namada::types::address::EstablishedAddressGen;
+    use namada::types::address::{
+        gen_established_address, EstablishedAddressGen,
+    };
     use namada::types::storage::{BlockHash, Epoch, Epochs};
     use tempfile::tempdir;
     use test_log::test;
@@ -1678,7 +1729,15 @@ mod test {
         )
         .unwrap();
 
-        add_block_to_batch(&db, &mut batch, BlockHeight::default()).unwrap();
+        add_block_to_batch(
+            &db,
+            &mut batch,
+            BlockHeight::default(),
+            Epoch::default(),
+            Epochs::default(),
+            &ConversionState::default(),
+        )
+        .unwrap();
         db.exec_batch(batch.0).unwrap();
 
         let _state = db
@@ -1852,6 +1911,12 @@ mod test {
         // Write first block
         let mut batch = RocksDB::batch();
         let height_0 = BlockHeight(100);
+        let mut pred_epochs = Epochs::default();
+        pred_epochs.new_epoch(height_0);
+        let mut conversion_state_0 = ConversionState::default();
+        conversion_state_0
+            .tokens
+            .insert("dummy1".to_string(), gen_established_address("test"));
         let to_delete_val = vec![1_u8, 1, 0, 0];
         let to_overwrite_val = vec![1_u8, 1, 1, 0];
         db.batch_write_subspace_val(
@@ -1869,12 +1934,25 @@ mod test {
         )
         .unwrap();
 
-        add_block_to_batch(&db, &mut batch, height_0).unwrap();
+        add_block_to_batch(
+            &db,
+            &mut batch,
+            height_0,
+            Epoch(1),
+            pred_epochs.clone(),
+            &conversion_state_0,
+        )
+        .unwrap();
         db.exec_batch(batch.0).unwrap();
 
         // Write second block
         let mut batch = RocksDB::batch();
         let height_1 = BlockHeight(101);
+        pred_epochs.new_epoch(height_1);
+        let mut conversion_state_1 = ConversionState::default();
+        conversion_state_1
+            .tokens
+            .insert("dummy2".to_string(), gen_established_address("test"));
         let add_val = vec![1_u8, 0, 0, 0];
         let overwrite_val = vec![1_u8, 1, 1, 1];
         db.batch_write_subspace_val(&mut batch, height_1, &add_key, &add_val)
@@ -1889,7 +1967,15 @@ mod test {
         db.batch_delete_subspace_val(&mut batch, height_1, &delete_key)
             .unwrap();
 
-        add_block_to_batch(&db, &mut batch, height_1).unwrap();
+        add_block_to_batch(
+            &db,
+            &mut batch,
+            height_1,
+            Epoch(2),
+            pred_epochs,
+            &conversion_state_1,
+        )
+        .unwrap();
         db.exec_batch(batch.0).unwrap();
 
         // Check that the values are as expected from second block
@@ -1910,20 +1996,28 @@ mod test {
         assert_eq!(overwritten, Some(to_overwrite_val));
         let deleted = db.read_subspace_val(&delete_key).unwrap();
         assert_eq!(deleted, Some(to_delete_val));
+        // Check the conversion state
+        let state_cf = db.get_column_family(STATE_CF).unwrap();
+        let conversion_state =
+            db.0.get_cf(state_cf, "conversion_state".as_bytes())
+                .unwrap()
+                .unwrap();
+        assert_eq!(conversion_state, types::encode(&conversion_state_0));
     }
 
     /// A test helper to write a block
     fn add_block_to_batch(
         db: &RocksDB,
         batch: &mut RocksDBWriteBatch,
         height: BlockHeight,
+        epoch: Epoch,
+        pred_epochs: Epochs,
+        conversion_state: &ConversionState,
     ) -> Result<()> {
         let merkle_tree = MerkleTree::<Sha256Hasher>::default();
         let merkle_tree_stores = merkle_tree.stores();
         let hash = BlockHash::default();
         let time = DateTimeUtc::now();
-        let epoch = Epoch::default();
-        let pred_epochs = Epochs::default();
         let next_epoch_min_start_height = BlockHeight::default();
         let next_epoch_min_start_time = DateTimeUtc::now();
         let update_epoch_blocks_delay = None;
@@ -1939,6 +2033,7 @@ mod test {
             time,
             epoch,
             results: &results,
+            conversion_state,
             pred_epochs: &pred_epochs,
             next_epoch_min_start_height,
             next_epoch_min_start_time,