Merge branch 'grarco/streamline-validation' (#3448)

* grarco/streamline-validation:
  Refactors recheck of `process_proposal` before `finalize_block`
  Changelog #3448
  Clippy + fmt
  Removes redundant check in `finalize_block` and implements recheck
  Updates docs of `finalize_block` and `process_proposal`
  Moves allowed txs check in `process_proposal` before the check on fees
  Removes duplicated validation in `prepare_proposal`
  Anticipates check on tx allowlist in mempool

.changelog/unreleased/improvements/3448-streamline-validation.md

@@ -0,0 +1,3 @@
+- Introduced a local configuration parameter to allow nodes to
+  rerun the process proposal checks before block finalization.
+  ([\#3448](https://github.com/anoma/namada/pull/3448))

crates/apps/src/bin/namada-node/cli.rs

@@ -5,7 +5,9 @@ use namada::core::time::{DateTimeUtc, Utc};
 use namada::sdk::migrations::ScheduledMigration;
 use namada_apps_lib::cli::cmds::TestGenesis;
 use namada_apps_lib::cli::{self, cmds};
-use namada_apps_lib::config::{Action, ActionAtHeight, ValidatorLocalConfig};
+use namada_apps_lib::config::{
+    Action, ActionAtHeight, NodeLocalConfig, ValidatorLocalConfig,
+};
 use namada_node as node;
 #[cfg(not(feature = "migrations"))]
 use namada_sdk::display_line;
@@ -127,7 +129,9 @@ pub fn main() -> Result<()> {
                     );
                 }
             }
-            cmds::Config::UpdateLocalConfig(cmds::LocalConfig(args)) => {
+            cmds::Config::UpdateValidatorLocalConfig(
+                cmds::ValidatorLocalConfig(args),
+            ) => {
                 // Validate the new config
                 let updated_config = std::fs::read(args.config_path).unwrap();
                 let _validator_local_config: ValidatorLocalConfig =
@@ -144,6 +148,23 @@ pub fn main() -> Result<()> {
                     .join("validator_local_config.toml");
                 std::fs::write(config_path, updated_config).unwrap();
             }
+            cmds::Config::UpdateLocalConfig(cmds::LocalConfig(args)) => {
+                // Validate the new config
+                let updated_config = std::fs::read(args.config_path).unwrap();
+                let _local_config: NodeLocalConfig =
+                    toml::from_slice(&updated_config).unwrap();
+
+                // Update the configuration file with the new one
+                let config_path = ctx
+                    .global_args
+                    .base_dir
+                    .join(format!(
+                        "{}",
+                        ctx.chain.unwrap().config.ledger.chain_id
+                    ))
+                    .join("local_config.toml");
+                std::fs::write(config_path, updated_config).unwrap();
+            }
         },
         cmds::NamadaNode::Utils(sub) => match sub {
             cmds::NodeUtils::TestGenesis(TestGenesis(args)) => {

crates/apps_lib/src/cli.rs

@@ -1071,6 +1071,7 @@ pub mod cmds {
     #[derive(Clone, Debug)]
     pub enum Config {
         Gen(ConfigGen),
+        UpdateValidatorLocalConfig(ValidatorLocalConfig),
         UpdateLocalConfig(LocalConfig),
     }
 
@@ -1080,9 +1081,11 @@ pub mod cmds {
         fn parse(matches: &ArgMatches) -> Option<Self> {
             matches.subcommand_matches(Self::CMD).and_then(|matches| {
                 let gen = SubCmd::parse(matches).map(Self::Gen);
-                let gas_tokens =
+                let validator_cfg = SubCmd::parse(matches)
+                    .map(Self::UpdateValidatorLocalConfig);
+                let local_cfg =
                     SubCmd::parse(matches).map(Self::UpdateLocalConfig);
-                gen.or(gas_tokens)
+                gen.or(validator_cfg).or(local_cfg)
             })
         }
 
@@ -1092,6 +1095,7 @@ pub mod cmds {
                 .arg_required_else_help(true)
                 .about(wrap!("Configuration sub-commands."))
                 .subcommand(ConfigGen::def())
+                .subcommand(ValidatorLocalConfig::def())
                 .subcommand(LocalConfig::def())
         }
     }
@@ -1112,6 +1116,25 @@ pub mod cmds {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct ValidatorLocalConfig(pub args::UpdateValidatorLocalConfig);
+
+    impl SubCmd for ValidatorLocalConfig {
+        const CMD: &'static str = "update-validator-local-config";
+
+        fn parse(matches: &ArgMatches) -> Option<Self> {
+            matches.subcommand_matches(Self::CMD).map(|matches| {
+                Self(args::UpdateValidatorLocalConfig::parse(matches))
+            })
+        }
+
+        fn def() -> App {
+            App::new(Self::CMD)
+                .about(wrap!("Update the validator's local configuration."))
+                .add_args::<args::UpdateValidatorLocalConfig>()
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct LocalConfig(pub args::UpdateLocalConfig);
 
@@ -1126,7 +1149,7 @@ pub mod cmds {
 
         fn def() -> App {
             App::new(Self::CMD)
-                .about(wrap!("Update the validator's local configuration."))
+                .about(wrap!("Update the node's local configuration."))
                 .add_args::<args::UpdateLocalConfig>()
         }
     }
@@ -3698,6 +3721,25 @@ pub mod args {
         }
     }
 
+    #[derive(Clone, Debug)]
+    pub struct UpdateValidatorLocalConfig {
+        pub config_path: PathBuf,
+    }
+
+    impl Args for UpdateValidatorLocalConfig {
+        fn parse(matches: &ArgMatches) -> Self {
+            let config_path = DATA_PATH.parse(matches);
+            Self { config_path }
+        }
+
+        fn def(app: App) -> App {
+            app.arg(DATA_PATH.def().help(wrap!(
+                "The path to the toml file containing the updated validator's \
+                 local configuration."
+            )))
+        }
+    }
+
     #[derive(Clone, Debug)]
     pub struct UpdateLocalConfig {
         pub config_path: PathBuf,
@@ -3711,8 +3753,8 @@ pub mod args {
 
         fn def(app: App) -> App {
             app.arg(DATA_PATH.def().help(wrap!(
-                "The path to the toml file containing the updated local \
-                 configuration."
+                "The path to the toml file containing the updated node's \
+                 local configuration."
             )))
         }
     }

crates/apps_lib/src/config/mod.rs

@@ -49,6 +49,11 @@ pub struct ValidatorLocalConfig {
         HashMap<namada::core::address::Address, namada::core::token::Amount>,
 }
 
+#[derive(Debug, Serialize, Deserialize)]
+pub struct NodeLocalConfig {
+    pub recheck_process_proposal: bool,
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub enum TendermintMode {
     Full,

crates/node/src/lib.rs

@@ -35,9 +35,12 @@ use futures::future::TryFutureExt;
 use namada::core::storage::BlockHeight;
 use namada::core::time::DateTimeUtc;
 use namada::eth_bridge::ethers::providers::{Http, Provider};
+use namada::key::tm_raw_hash_to_string;
+use namada::ledger::pos::find_validator_by_raw_hash;
 use namada::state::DB;
 use namada::storage::DbColFam;
 use namada::tendermint::abci::request::CheckTxKind;
+use namada::tx::data::ResultCode;
 use namada_apps_lib::cli::args;
 use namada_apps_lib::config::utils::{
     convert_tm_addr_to_socket_addr, num_of_threads,
@@ -134,6 +137,8 @@ impl Shell {
             }
             Request::FinalizeBlock(finalize) => {
                 tracing::debug!("Request FinalizeBlock");
+
+                self.try_recheck_process_proposal(&finalize)?;
                 self.finalize_block(finalize).map(Response::FinalizeBlock)
             }
             Request::Commit => {
@@ -166,6 +171,61 @@ impl Shell {
             }
         }
     }
+
+    // Checks if a run of process proposal is required before finalize block
+    // (recheck) and, in case, performs it
+    fn try_recheck_process_proposal(
+        &self,
+        finalize_req: &shims::abcipp_shim_types::shim::request::FinalizeBlock,
+    ) -> Result<(), Error> {
+        let recheck_process_proposal = match self.mode {
+            shell::ShellMode::Validator {
+                ref local_config, ..
+            } => local_config
+                .as_ref()
+                .map(|cfg| cfg.recheck_process_proposal)
+                .unwrap_or_default(),
+            shell::ShellMode::Full { ref local_config } => local_config
+                .as_ref()
+                .map(|cfg| cfg.recheck_process_proposal)
+                .unwrap_or_default(),
+            shell::ShellMode::Seed => false,
+        };
+
+        if recheck_process_proposal {
+            let tm_raw_hash_string =
+                tm_raw_hash_to_string(&finalize_req.proposer_address);
+            let block_proposer =
+                find_validator_by_raw_hash(&self.state, tm_raw_hash_string)
+                    .unwrap()
+                    .expect(
+                        "Unable to find native validator address of block \
+                         proposer from tendermint raw hash",
+                    );
+
+            let check_result = self.process_txs(
+                &finalize_req
+                    .txs
+                    .iter()
+                    .map(|ptx| ptx.tx.clone())
+                    .collect::<Vec<_>>(),
+                finalize_req.header.time,
+                &block_proposer,
+            );
+
+            let invalid_txs = check_result.iter().any(|res| {
+                let error = ResultCode::from_u32(res.code)
+                    .expect("Failed to cast result code");
+                !error.is_recoverable()
+            });
+
+            if invalid_txs {
+                return Err(Error::InvalidBlockProposal);
+            }
+        }
+
+        Ok(())
+    }
 }
 
 /// Determine if the ledger is migrating state.

crates/node/src/shell/finalize_block.rs

@@ -44,22 +44,7 @@ where
     /// of epoch changes and applies associated updates to validator sets,
     /// etc. as necessary.
     ///
-    /// Validate and apply decrypted transactions unless
-    /// [`Shell::process_proposal`] detected that they were not submitted in
-    /// correct order or more decrypted txs arrived than expected. In that
-    /// case, all decrypted transactions are not applied and must be
-    /// included in the next `Shell::prepare_proposal` call.
-    ///
-    /// Incoming wrapper txs need no further validation. They
-    /// are added to the block.
-    ///
-    /// Error codes:
-    ///   0: Ok
-    ///   1: Invalid tx
-    ///   2: Tx is invalidly signed
-    ///   3: Wasm runtime error
-    ///   4: Invalid order of decrypted txs
-    ///   5. More decrypted txs than expected
+    /// Apply the transactions included in the block.
     pub fn finalize_block(
         &mut self,
         req: shim::request::FinalizeBlock,
@@ -668,13 +653,6 @@ where
                 continue;
             }
 
-            if tx.validate_tx().is_err() {
-                tracing::error!(
-                    "Internal logic error: FinalizeBlock received tx that \
-                     could not be deserialized to a valid TxType"
-                );
-                continue;
-            };
             let tx_header = tx.header();
             // If [`process_proposal`] rejected a Tx, emit an event here and
             // move on to next tx