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

* origin/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