Merge branch 'tomas/protocol-write-log' (#1108) into maint-0.13

* tomas/protocol-write-log:
  [ci] wasm checksums update
  changelog: add #1108
  shell/queries: refactor token query using storage_api token mod
  shell/gov: apply changes via `WlStorage` write log
  test/finalize_block: ensure that proposal doesn't commit to DB
  gov + token: refactor using storage_api
  move gov mod from tx_prelude to core storage_api
  test/test_vp_host_env: update to write via tx
  ledger: apply `update_allowed_conversions` storage changes via WlStorage
  wl_storage: apply protocol write log changes at block level
  test/ledger/shell: test that `finalize_block` doesn't commit storage

.changelog/unreleased/improvements/1108-protocol-write-log.md

@@ -0,0 +1,4 @@
+- Improved the `WlStorage` to write protocol changes via block-level write log.
+  This is then used to make sure that no storage changes are committed in ABCI
+  `FinalizeBlock` request handler and only in the `Commit` handler.
+  ([#1108](https://github.com/anoma/namada/pull/1108))

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

@@ -50,6 +50,13 @@ where
             self.update_state(req.header, req.hash, req.byzantine_validators);
 
         if new_epoch {
+            if let Err(e) = namada::ledger::storage::update_allowed_conversions(
+                &mut self.wl_storage,
+            ) {
+                tracing::error!(
+                    "Failed to update allowed conversions with {e}"
+                );
+            }
             let _proposals_result =
                 execute_governance_proposals(self, &mut response)?;
         }
@@ -436,7 +443,17 @@ where
 /// are covered by the e2e tests.
 #[cfg(test)]
 mod test_finalize_block {
+    use std::collections::BTreeMap;
+    use std::str::FromStr;
+
+    use namada::ledger::parameters::EpochDuration;
+    use namada::ledger::storage_api;
+    use namada::types::governance::ProposalVote;
     use namada::types::storage::Epoch;
+    use namada::types::time::DurationSecs;
+    use namada::types::transaction::governance::{
+        InitProposalData, VoteProposalData,
+    };
     use namada::types::transaction::{EncryptionKey, Fee, WrapperTx, MIN_FEE};
 
     use super::*;
@@ -789,4 +806,96 @@ mod test_finalize_block {
         }
         assert_eq!(counter, 2);
     }
+
+    /// Test that the finalize block handler never commits changes directly to
+    /// the DB.
+    #[test]
+    fn test_finalize_doesnt_commit_db() {
+        let (mut shell, _) = setup();
+
+        // Update epoch duration to make sure we go through couple epochs
+        let epoch_duration = EpochDuration {
+            min_num_of_blocks: 5,
+            min_duration: DurationSecs(0),
+        };
+        namada::ledger::parameters::update_epoch_parameter(
+            &mut shell.wl_storage.storage,
+            &epoch_duration,
+        )
+        .unwrap();
+        shell.wl_storage.storage.next_epoch_min_start_height = BlockHeight(5);
+        shell.wl_storage.storage.next_epoch_min_start_time = DateTimeUtc::now();
+
+        // Add a proposal to be executed on next epoch change.
+        let mut add_proposal = |proposal_id, vote| {
+            let validator = shell.mode.get_validator_address().unwrap().clone();
+            shell.proposal_data.insert(proposal_id);
+            let proposal = InitProposalData {
+                id: Some(proposal_id),
+                content: vec![],
+                author: validator.clone(),
+                voting_start_epoch: Epoch::default(),
+                voting_end_epoch: Epoch::default().next(),
+                grace_epoch: Epoch::default().next(),
+                proposal_code: None,
+            };
+            storage_api::governance::init_proposal(
+                &mut shell.wl_storage,
+                proposal,
+            )
+            .unwrap();
+            let vote = VoteProposalData {
+                id: proposal_id,
+                vote,
+                voter: validator,
+                delegations: vec![],
+            };
+            // Vote to accept the proposal (there's only one validator, so its
+            // vote decides)
+            storage_api::governance::vote_proposal(&mut shell.wl_storage, vote)
+                .unwrap();
+        };
+        // Add a proposal to be accepted and one to be rejected.
+        add_proposal(0, ProposalVote::Yay);
+        add_proposal(1, ProposalVote::Nay);
+
+        // Commit the genesis state
+        shell.wl_storage.commit_genesis().unwrap();
+        shell.commit();
+
+        // Collect all storage key-vals into a sorted map
+        let store_block_state = |shell: &TestShell| -> BTreeMap<_, _> {
+            let prefix: Key = FromStr::from_str("").unwrap();
+            shell
+                .wl_storage
+                .storage
+                .db
+                .iter_prefix(&prefix)
+                .map(|(key, val, _gas)| (key, val))
+                .collect()
+        };
+
+        // Store the full state in sorted map
+        let mut last_storage_state: std::collections::BTreeMap<
+            String,
+            Vec<u8>,
+        > = store_block_state(&shell);
+
+        // Keep applying finalize block
+        for _ in 0..20 {
+            let req = FinalizeBlock::default();
+            let _events = shell.finalize_block(req).unwrap();
+            let new_state = store_block_state(&shell);
+            // The new state must be unchanged
+            itertools::assert_equal(
+                last_storage_state.iter(),
+                new_state.iter(),
+            );
+            // Commit the block to move on to the next one
+            shell.wl_storage.commit_block().unwrap();
+
+            // Store the state after commit for the next iteration
+            last_storage_state = store_block_state(&shell);
+        }
+    }
 }

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

@@ -9,10 +9,10 @@ use namada::ledger::native_vp::governance::utils::{
 use namada::ledger::protocol;
 use namada::ledger::storage::types::encode;
 use namada::ledger::storage::{DBIter, StorageHasher, DB};
+use namada::ledger::storage_api::{token, StorageWrite};
 use namada::types::address::Address;
 use namada::types::governance::TallyResult;
 use namada::types::storage::Epoch;
-use namada::types::token;
 
 use super::*;
 
@@ -84,8 +84,7 @@ where
                             gov_storage::get_proposal_execution_key(id);
                         shell
                             .wl_storage
-                            .storage
-                            .write(&pending_execution_key, "")
+                            .write(&pending_execution_key, ())
                             .expect("Should be able to write to storage.");
                         let tx_result = protocol::apply_tx(
                             tx_type,
@@ -101,7 +100,6 @@ where
                         );
                         shell
                             .wl_storage
-                            .storage
                             .delete(&pending_execution_key)
                             .expect("Should be able to delete the storage.");
                         match tx_result {
@@ -206,11 +204,16 @@ where
 
         let native_token = shell.wl_storage.storage.native_token.clone();
         // transfer proposal locked funds
-        shell.wl_storage.transfer(
+        token::transfer(
+            &mut shell.wl_storage,
             &native_token,
-            funds,
             &gov_address,
             &transfer_address,
+            funds,
+        )
+        .expect(
+            "Must be able to transfer governance locked funds after proposal \
+             has been tallied",
         );
     }
 

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

@@ -4,10 +4,10 @@ use borsh::BorshSerialize;
 use ferveo_common::TendermintValidator;
 use namada::ledger::pos::into_tm_voting_power;
 use namada::ledger::queries::{RequestCtx, ResponseQuery};
-use namada::ledger::storage_api;
+use namada::ledger::storage_api::token;
 use namada::types::address::Address;
+use namada::types::key;
 use namada::types::key::dkg_session_keys::DkgPublicKey;
-use namada::types::{key, token};
 
 use super::*;
 use crate::node::ledger::response;
@@ -69,15 +69,10 @@ where
         token: &Address,
         owner: &Address,
     ) -> token::Amount {
-        let balance = storage_api::StorageRead::read(
-            &self.wl_storage,
-            &token::balance_key(token, owner),
-        );
         // Storage read must not fail, but there might be no value, in which
         // case default (0) is returned
-        balance
-            .expect("Storage read in the protocol must not fail")
-            .unwrap_or_default()
+        token::read_balance(&self.wl_storage, token, owner)
+            .expect("Token balance read in the protocol must not fail")
     }
 
     /// Lookup data about a validator from their protocol signing key