fix: e2e working with aggregation 2/2 of message_id and merkle_root

rust/agents/relayer/src/merkle_tree/builder.rs

@@ -3,7 +3,7 @@ use std::fmt::Display;
 use eyre::{Context, Result};
 use tracing::{debug, error, instrument};
 
-use hyperlane_base::db::{DbError, HyperlaneRocksDB};
+use hyperlane_base::db::DbError;
 use hyperlane_core::{
     accumulator::{incremental::IncrementalMerkle, merkle::Proof},
     ChainCommunicationError, H256,
@@ -14,7 +14,6 @@ use crate::prover::{Prover, ProverError};
 /// Struct to sync prover.
 #[derive(Debug)]
 pub struct MerkleTreeBuilder {
-    db: HyperlaneRocksDB,
     prover: Prover,
     incremental: IncrementalMerkle,
 }
@@ -62,36 +61,24 @@ pub enum MerkleTreeBuilderError {
 }
 
 impl MerkleTreeBuilder {
-    pub fn new(db: HyperlaneRocksDB) -> Self {
+    pub fn new() -> Self {
         let prover = Prover::default();
         let incremental = IncrementalMerkle::default();
         Self {
             prover,
             incremental,
-            db,
         }
     }
 
     #[instrument(err, skip(self), level="debug", fields(prover_latest_index=self.count()-1))]
     pub fn get_proof(
         &self,
-        message_nonce: u32,
+        leaf_index: u32,
         root_index: u32,
-    ) -> Result<Option<Proof>, MerkleTreeBuilderError> {
-        self.db
-            .retrieve_message_id_by_nonce(&message_nonce)?
-            .and_then(|message_id| {
-                self.db
-                    .retrieve_merkle_leaf_index_by_message_id(&message_id)
-                    .ok()
-                    .flatten()
-            })
-            .map(|leaf_index| {
-                self.prover
-                    .prove_against_previous(leaf_index as usize, root_index as usize)
-            })
-            .transpose()
-            .map_err(Into::into)
+    ) -> Result<Proof, MerkleTreeBuilderError> {
+        self.prover
+            .prove_against_previous(leaf_index as usize, root_index as usize)
+            .map_err(MerkleTreeBuilderError::from)
     }
 
     pub fn count(&self) -> u32 {

rust/agents/relayer/src/msg/metadata/aggregation.rs

@@ -4,11 +4,13 @@ use futures_util::future::join_all;
 
 use derive_new::new;
 use eyre::Context;
+use itertools::{Either, Itertools};
 use tracing::{info, instrument};
 
-use hyperlane_core::{HyperlaneMessage, InterchainSecurityModule, H256, U256};
+use hyperlane_core::{HyperlaneMessage, InterchainSecurityModule, ModuleType, H256, U256};
 
-use super::{BaseMetadataBuilder, MetadataBuilder};
+use super::{base::IsmWithMetadataAndType, BaseMetadataBuilder, MetadataBuilder};
+use eyre::ErrReport;
 
 /// Bytes used to store one member of the (start, end) range tuple
 /// Copied from `AggregationIsmMetadata.sol`
@@ -90,15 +92,15 @@ impl AggregationIsmMetadataBuilder {
         sub_modules: Vec<IsmAndMetadata>,
         message: &HyperlaneMessage,
         threshold: usize,
+        err_isms: Vec<(H256, Option<ModuleType>)>,
     ) -> Option<Vec<SubModuleMetadata>> {
         let gas_cost_results: Vec<_> = join_all(
             sub_modules
                 .iter()
                 .map(|module| module.ism.dry_run_verify(message, &(module.meta.metadata))),
         )
         .await;
-
-        // Filter out the ISMs without a gas cost estimate
+        // Filter out the ISMs with a gas cost estimate
         let metas_and_gas: Vec<_> = sub_modules
             .into_iter()
             .zip(gas_cost_results.into_iter())
@@ -107,7 +109,7 @@ impl AggregationIsmMetadataBuilder {
 
         let metas_and_gas_count = metas_and_gas.len();
         if metas_and_gas_count < threshold {
-            info!("Could not fetch all metadata: Found {metas_and_gas_count} of the {threshold} required ISM metadata pieces");
+            info!(?err_isms, %metas_and_gas_count, %threshold, message_id=message.id().to_string(), "Could not fetch all metadata, ISM metadata count did not reach aggregation threshold");
             return None;
         }
         Some(Self::n_cheapest_metas(metas_and_gas, threshold))
@@ -127,34 +129,33 @@ impl MetadataBuilder for AggregationIsmMetadataBuilder {
         let (ism_addresses, threshold) = ism.modules_and_threshold(message).await.context(CTX)?;
         let threshold = threshold as usize;
 
-        let metas = join_all(
-            ism_addresses
-                .iter()
-                .map(|ism_address| self.base.build(*ism_address, message)),
-        )
-        .await;
-
-        let sub_modules = join_all(
+        let sub_modules_and_metas = join_all(
             ism_addresses
                 .iter()
-                .map(|ism_address| self.base.build_ism(*ism_address)),
+                .map(|ism_address| self.base.build_ism_and_metadata(*ism_address, message)),
         )
         .await;
 
-        let filtered_sub_module_metas = metas
+        // Partitions things into
+        // 1. ok_sub_modules: ISMs with metadata with valid metadata
+        // 2. err_sub_modules: ISMs with invalid metadata
+        let (ok_sub_modules, err_sub_modules): (Vec<_>, Vec<_>) = sub_modules_and_metas
             .into_iter()
+            .zip(ism_addresses.iter())
             .enumerate()
-            .zip(sub_modules.into_iter())
-            .filter_map(|((index, meta_result), sub_module_result)| {
-                match (meta_result, sub_module_result) {
-                    (Ok(Some(meta)), Ok(ism)) => Some(IsmAndMetadata::new(ism, index, meta)),
-                    _ => None,
-                }
-            })
-            .collect();
-
+            .partition_map(|(index, (result, ism_address))| match result {
+                Ok(sub_module_and_meta) => match sub_module_and_meta.metadata {
+                    Some(metadata) => Either::Left(IsmAndMetadata::new(
+                        sub_module_and_meta.ism,
+                        index,
+                        metadata,
+                    )),
+                    None => Either::Right((*ism_address, Some(sub_module_and_meta.module_type))),
+                },
+                Err(_) => Either::Right((*ism_address, None)),
+            });
         let maybe_aggregation_metadata =
-            Self::cheapest_valid_metas(filtered_sub_module_metas, message, threshold)
+            Self::cheapest_valid_metas(ok_sub_modules, message, threshold, err_sub_modules)
                 .await
                 .map(|mut metas| Self::format_metadata(&mut metas, ism_addresses.len()));
         Ok(maybe_aggregation_metadata)

rust/agents/relayer/src/msg/metadata/base.rs

@@ -1,5 +1,13 @@
 use std::{collections::HashMap, fmt::Debug, str::FromStr, sync::Arc};
 
+use crate::{
+    merkle_tree::builder::MerkleTreeBuilder,
+    msg::metadata::{
+        multisig::{MerkleRootMultisigMetadataBuilder, MessageIdMultisigMetadataBuilder},
+        AggregationIsmMetadataBuilder, CcipReadIsmMetadataBuilder, NullMetadataBuilder,
+        RoutingIsmMetadataBuilder,
+    },
+};
 use async_trait::async_trait;
 use derive_new::new;
 use eyre::{Context, Result};
@@ -16,15 +24,6 @@ use hyperlane_core::{
 use tokio::sync::RwLock;
 use tracing::{debug, info, instrument, warn};
 
-use crate::{
-    merkle_tree::builder::MerkleTreeBuilder,
-    msg::metadata::{
-        multisig::{MerkleRootMultisigMetadataBuilder, MessageIdMultisigMetadataBuilder},
-        AggregationIsmMetadataBuilder, CcipReadIsmMetadataBuilder, NullMetadataBuilder,
-        RoutingIsmMetadataBuilder,
-    },
-};
-
 #[derive(Debug, thiserror::Error)]
 pub enum MetadataBuilderError {
     #[error("Unknown or invalid module type ({0})")]
@@ -33,6 +32,12 @@ pub enum MetadataBuilderError {
     MaxDepthExceeded(u32),
 }
 
+pub struct IsmWithMetadataAndType {
+    pub ism: Box<dyn InterchainSecurityModule>,
+    pub metadata: Option<Vec<u8>>,
+    pub module_type: ModuleType,
+}
+
 #[async_trait]
 pub trait MetadataBuilder: Send + Sync {
     #[allow(clippy::async_yields_async)]
@@ -73,31 +78,9 @@ impl MetadataBuilder for BaseMetadataBuilder {
         ism_address: H256,
         message: &HyperlaneMessage,
     ) -> Result<Option<Vec<u8>>> {
-        let ism = self
-            .build_ism(ism_address)
-            .await
-            .context("When building ISM")?;
-        let module_type = ism
-            .module_type()
-            .await
-            .context("When fetching module type")?;
-        let base = self.clone_with_incremented_depth()?;
-
-        let metadata_builder: Box<dyn MetadataBuilder> = match module_type {
-            ModuleType::MerkleRootMultisig => {
-                Box::new(MerkleRootMultisigMetadataBuilder::new(base))
-            }
-            ModuleType::MessageIdMultisig => Box::new(MessageIdMultisigMetadataBuilder::new(base)),
-            ModuleType::Routing => Box::new(RoutingIsmMetadataBuilder::new(base)),
-            ModuleType::Aggregation => Box::new(AggregationIsmMetadataBuilder::new(base)),
-            ModuleType::Null => Box::new(NullMetadataBuilder::new()),
-            ModuleType::CcipRead => Box::new(CcipReadIsmMetadataBuilder::new(base)),
-            _ => return Err(MetadataBuilderError::UnsupportedModuleType(module_type).into()),
-        };
-        metadata_builder
-            .build(ism_address, message)
+        self.build_ism_and_metadata(ism_address, message)
             .await
-            .context("When building metadata")
+            .map(|ism_with_metadata| ism_with_metadata.metadata)
     }
 }
 
@@ -116,26 +99,22 @@ impl BaseMetadataBuilder {
         }
     }
 
-    pub async fn get_proof(&self, nonce: u32, checkpoint: Checkpoint) -> Result<Option<Proof>> {
+    pub async fn get_proof(&self, leaf_index: u32, checkpoint: Checkpoint) -> Result<Proof> {
         const CTX: &str = "When fetching message proof";
-        let proof = self.origin_prover_sync
+        let proof = self
+            .origin_prover_sync
             .read()
             .await
-            .get_proof(nonce, checkpoint.index)
-            .context(CTX)?
-            .and_then(|proof| {
-                // checkpoint may be fraudulent if the root does not
-                // match the canonical root at the checkpoint's index
-                if proof.root() == checkpoint.root {
-                    return Some(proof)
-                }
-                info!(
-                    ?checkpoint,
-                    canonical_root = ?proof.root(),
-                    "Could not fetch metadata: checkpoint root does not match canonical root from merkle proof"
-                );
-                None
-            });
+            .get_proof(leaf_index, checkpoint.index)
+            .context(CTX)?;
+
+        if proof.root() != checkpoint.root {
+            info!(
+                ?checkpoint,
+                canonical_root = ?proof.root(),
+                "Could not fetch metadata: checkpoint root does not match canonical root from merkle proof"
+            );
+        }
         Ok(proof)
     }
 
@@ -244,4 +223,43 @@ impl BaseMetadataBuilder {
         }
         Ok(MultisigCheckpointSyncer::new(checkpoint_syncers))
     }
+
+    #[instrument(err, skip(self), fields(domain=self.domain().name()))]
+    pub async fn build_ism_and_metadata(
+        &self,
+        ism_address: H256,
+        message: &HyperlaneMessage,
+    ) -> Result<IsmWithMetadataAndType> {
+        let ism: Box<dyn InterchainSecurityModule> = self
+            .build_ism(ism_address)
+            .await
+            .context("When building ISM")?;
+
+        let module_type = ism
+            .module_type()
+            .await
+            .context("When fetching module type")?;
+        let base = self.clone_with_incremented_depth()?;
+
+        let metadata_builder: Box<dyn MetadataBuilder> = match module_type {
+            ModuleType::MerkleRootMultisig => {
+                Box::new(MerkleRootMultisigMetadataBuilder::new(base))
+            }
+            ModuleType::MessageIdMultisig => Box::new(MessageIdMultisigMetadataBuilder::new(base)),
+            ModuleType::Routing => Box::new(RoutingIsmMetadataBuilder::new(base)),
+            ModuleType::Aggregation => Box::new(AggregationIsmMetadataBuilder::new(base)),
+            ModuleType::Null => Box::new(NullMetadataBuilder::new()),
+            ModuleType::CcipRead => Box::new(CcipReadIsmMetadataBuilder::new(base)),
+            _ => return Err(MetadataBuilderError::UnsupportedModuleType(module_type).into()),
+        };
+        let meta = metadata_builder
+            .build(ism_address, message)
+            .await
+            .context("When building metadata");
+        Ok(IsmWithMetadataAndType {
+            ism,
+            metadata: meta?,
+            module_type,
+        })
+    }
 }

rust/agents/relayer/src/msg/metadata/multisig/merkle_root_multisig.rs

@@ -67,13 +67,10 @@ impl MultisigIsmMetadataBuilder for MerkleRootMultisigMetadataBuilder {
                 highest_leaf_index, "Couldn't get checkpoint in range"
             )
         );
-        unwrap_or_none_result!(
-            proof,
-            self.get_proof(leaf_index, quorum_checkpoint.checkpoint.checkpoint)
-                .await
-                .context(CTX)?,
-            debug!(leaf_index, checkpoint=?quorum_checkpoint, "Couldn't get proof")
-        );
+        let proof = self
+            .get_proof(leaf_index, quorum_checkpoint.checkpoint.checkpoint)
+            .await
+            .context(CTX)?;
         Ok(Some(MultisigMetadata::new(
             quorum_checkpoint,
             leaf_index,

rust/agents/relayer/src/msg/processor.rs

@@ -251,7 +251,7 @@ mod test {
         let core_metrics = CoreMetrics::new("dummy_relayer", 37582, Registry::new()).unwrap();
         BaseMetadataBuilder::new(
             destination_chain_conf.clone(),
-            Arc::new(RwLock::new(MerkleTreeBuilder::new(db.clone()))),
+            Arc::new(RwLock::new(MerkleTreeBuilder::new())),
             Arc::new(MockValidatorAnnounceContract::default()),
             false,
             Arc::new(core_metrics),

rust/agents/relayer/src/relayer.rs

@@ -160,10 +160,9 @@ impl BaseAgent for Relayer {
             .origin_chains
             .iter()
             .map(|origin| {
-                let db = dbs.get(origin).unwrap().clone();
                 (
                     origin.clone(),
-                    Arc::new(RwLock::new(MerkleTreeBuilder::new(db))),
+                    Arc::new(RwLock::new(MerkleTreeBuilder::new())),
                 )
             })
             .collect::<HashMap<_, _>>();

rust/agents/validator/src/submit.rs

@@ -253,7 +253,6 @@ impl ValidatorSubmitter {
                 );
                 continue;
             }
-
             let signed_checkpoint = self.signer.sign(queued_checkpoint).await?;
             self.checkpoint_syncer
                 .write_checkpoint(&signed_checkpoint)

rust/chains/hyperlane-ethereum/tests/signer_output.rs

@@ -7,7 +7,6 @@ use std::{fs::OpenOptions, io::Write, str::FromStr};
 use hex::FromHex;
 use serde_json::{json, Value};
 
-use ethers::signers::Signer;
 use hyperlane_core::{
     accumulator::{
         merkle::{merkle_root_from_branch, MerkleTree},

rust/hyperlane-base/src/types/multisig.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use derive_new::new;
 use eyre::Result;
-use tracing::{debug, instrument, trace};
+use tracing::{debug, instrument};
 
 use hyperlane_core::{MultisigSignedCheckpoint, SignedCheckpointWithMessageId, H160, H256};
 
@@ -46,7 +46,7 @@ impl MultisigCheckpointSyncer {
                 // Gracefully handle errors getting the latest_index
                 match checkpoint_syncer.latest_index().await {
                     Ok(Some(index)) => {
-                        trace!(?address, ?index, "Validator returned latest index");
+                        debug!(?address, ?index, "Validator returned latest index");
                         latest_indices.push(index);
                     }
                     err => {

typescript/infra/config/environments/test/aggregationIsm.ts

@@ -9,6 +9,6 @@ export const aggregationIsm = (validatorKey: string): AggregationIsmConfig => {
       merkleRootMultisig(validatorKey),
       messageIdMultisig(validatorKey),
     ],
-    threshold: 1,
+    threshold: 2,
   };
 };