Skip to content

Commit

Permalink
refactor(relayer): improve aggregation ism readability
Browse files Browse the repository at this point in the history
  • Loading branch information
@daniel-savu
daniel-savu committed Jun 29, 2023
1 parent 4fe3a68 commit 022bd99
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 65 deletions.
187 changes: 122 additions & 65 deletions rust/agents/relayer/src/msg/metadata/aggregation.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use async_trait::async_trait;
use futures_util::future::{join_all, try_join};
use std::ops::Deref;
use derive_more::Deref;
use futures_util::future::join_all;

use derive_new::new;
use eyre::Context;
Expand All @@ -14,21 +14,36 @@ use super::{BaseMetadataBuilder, MetadataBuilder};
/// Copied from `AggregationIsmMetadata.sol`
const METADATA_RANGE_SIZE: usize = 4;

#[derive(Clone, Debug, new)]
#[derive(Clone, Debug, new, Deref)]
pub struct AggregationIsmMetadataBuilder {
base: BaseMetadataBuilder,
}

#[derive(Clone, Debug, new, PartialEq, Eq)]
struct SubIsmMetadata {
/// The index of the sub-ISM in the aggregation ISM.
struct SubModuleMetadata {
/// The index of the sub-module (ISM) in the aggregation ISM.
index: usize,
/// The metadata for the sub-ISM.
/// The metadata for the sub-module.
metadata: Vec<u8>,
}

#[derive(Debug)]
struct IsmAndMetadata {
ism: Box<dyn InterchainSecurityModule>,
meta: SubModuleMetadata,
}

impl IsmAndMetadata {
fn new(ism: Box<dyn InterchainSecurityModule>, index: usize, metadata: Vec<u8>) -> Self {
Self {
ism,
meta: SubModuleMetadata::new(index, metadata),
}
}
}

impl AggregationIsmMetadataBuilder {
fn format_metadata(metadatas: &mut [SubIsmMetadata], ism_count: usize) -> Vec<u8> {
fn format_metadata(metadatas: &mut [SubModuleMetadata], ism_count: usize) -> Vec<u8> {
// See test solidity implementation of this fn at:
// https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/445da4fb0d8140a08c4b314e3051b7a934b0f968/solidity/test/isms/AggregationIsm.t.sol#L35
fn encode_byte_index(i: usize) -> [u8; 4] {
Expand All @@ -41,7 +56,7 @@ impl AggregationIsmMetadataBuilder {
// Initialize the range tuple part of the buffer, so the actual metadatas can
// simply be appended to it
let mut buffer = vec![0; range_tuples_size];
for SubIsmMetadata { index, metadata } in metadatas.iter_mut() {
for SubModuleMetadata { index, metadata } in metadatas.iter_mut() {
let range_start = buffer.len();
buffer.append(metadata);
let range_end = buffer.len();
Expand All @@ -59,46 +74,43 @@ impl AggregationIsmMetadataBuilder {
}

fn n_cheapest_metas(
mut metas_and_gas: Vec<(SubIsmMetadata, U256)>,
mut metas_and_gas: Vec<(SubModuleMetadata, U256)>,
n: usize,
) -> Vec<SubIsmMetadata> {
) -> Vec<SubModuleMetadata> {
// Sort by gas cost in ascending order
metas_and_gas.sort_by(|(_, gas_1), (_, gas_2)| gas_1.cmp(gas_2));
// Take the cheapest n (the aggregation ISM threshold)
let mut cheapest: Vec<_> = metas_and_gas[..n].into();
// Sort by index in ascending order, to match the order expected by the smart contract
cheapest.sort_by(|(meta_1, _), (meta_2, _)| meta_1.index.cmp(&meta_2.index));
cheapest.iter().cloned().map(|(meta, _)| meta).collect()
}

async fn cheapest_valid_metas(
sub_isms: Vec<(SubIsmMetadata, Box<dyn InterchainSecurityModule>)>,
sub_modules: Vec<IsmAndMetadata>,
message: &HyperlaneMessage,
threshold: usize,
) -> eyre::Result<Option<Vec<SubIsmMetadata>>> {
let metas_and_gas: Vec<_> =
join_all(sub_isms.into_iter().map(|(sub_ism_meta, ism)| async move {
let gas = ism.dry_run_verify(message, &sub_ism_meta.metadata).await;
(sub_ism_meta, gas)
}))
.await
) -> 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
let metas_and_gas: Vec<_> = sub_modules
.into_iter()
.filter_map(|(sub_ism_meta, gast_result)| {
gast_result.ok().flatten().map(|gc| (sub_ism_meta, gc))
})
.zip(gas_cost_results.into_iter())
.filter_map(|(module, gas_cost)| gas_cost.ok().flatten().map(|gc| (module.meta, gc)))
.collect();

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");
return Ok(None);
return None;
}
Ok(Some(Self::n_cheapest_metas(metas_and_gas, threshold)))
}
}

impl Deref for AggregationIsmMetadataBuilder {
type Target = BaseMetadataBuilder;

fn deref(&self) -> &Self::Target {
&self.base
Some(Self::n_cheapest_metas(metas_and_gas, threshold))
}
}

Expand All @@ -110,31 +122,42 @@ impl MetadataBuilder for AggregationIsmMetadataBuilder {
ism_address: H256,
message: &HyperlaneMessage,
) -> eyre::Result<Option<Vec<u8>>> {
const CTX: &str = "When fetching RoutingIsm metadata";
const CTX: &str = "When fetching AggregationIsm metadata";
let ism = self.build_aggregation_ism(ism_address).await.context(CTX)?;
let (modules, threshold) = ism.modules_and_threshold(message).await.context(CTX)?;
let (ism_addresses, threshold) = ism.modules_and_threshold(message).await.context(CTX)?;
let threshold = threshold as usize;

let sub_isms: Vec<_> = join_all(modules.iter().map(|ism_address| {
try_join(
self.base.build(*ism_address, message),
self.base.build_ism(*ism_address),
)
}))
.await
.into_iter()
.enumerate()
.filter_map(|(i, r)| match r {
Ok((Some(meta), ism)) => Some((SubIsmMetadata::new(i, meta), ism)),
_ => None,
})
.collect();

Self::cheapest_valid_metas(sub_isms, message, threshold)
.await
.map(|maybe_proofs| {
maybe_proofs.map(|mut proofs| Self::format_metadata(&mut proofs, modules.len()))
let metas = join_all(
ism_addresses
.iter()
.map(|ism_address| self.base.build(*ism_address, message)),
)
.await;

let sub_modules = join_all(
ism_addresses
.iter()
.map(|ism_address| self.base.build_ism(*ism_address)),
)
.await;

let filtered_sub_module_metas = metas
.into_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();

let maybe_aggregation_metadata =
Self::cheapest_valid_metas(filtered_sub_module_metas, message, threshold)
.await
.map(|mut metas| Self::format_metadata(&mut metas, ism_addresses.len()));
Ok(maybe_aggregation_metadata)
}
}

Expand All @@ -147,17 +170,17 @@ mod test {
#[test]
fn test_format_n_of_n_metadata_works_correctly() {
let mut metadatas = vec![
SubIsmMetadata::new(
SubModuleMetadata::new(
0,
Vec::from_hex("290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563")
.unwrap(),
),
SubIsmMetadata::new(
SubModuleMetadata::new(
1,
Vec::from_hex("510e4e770828ddbf7f7b00ab00a9f6adaf81c0dc9cc85f1f8249c256942d61d9")
.unwrap(),
),
SubIsmMetadata::new(
SubModuleMetadata::new(
2,
Vec::from_hex("356e5a2cc1eba076e650ac7473fccc37952b46bc2e419a200cec0c451dce2336")
.unwrap(),
Expand All @@ -172,23 +195,24 @@ mod test {

#[test]
fn test_format_n_of_m_metadata_works_correctly() {
// We're passing the metadatas of 4 ISMs (indexes 0, 1, 2, 4) out of 5
let mut metadatas = vec![
SubIsmMetadata::new(
SubModuleMetadata::new(
0,
Vec::from_hex("290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563")
.unwrap(),
),
SubIsmMetadata::new(
SubModuleMetadata::new(
1,
Vec::from_hex("510e4e770828ddbf7f7b00ab00a9f6adaf81c0dc9cc85f1f8249c256942d61d9")
.unwrap(),
),
SubIsmMetadata::new(
SubModuleMetadata::new(
2,
Vec::from_hex("356e5a2cc1eba076e650ac7473fccc37952b46bc2e419a200cec0c451dce2336")
.unwrap(),
),
SubIsmMetadata::new(
SubModuleMetadata::new(
4,
Vec::from_hex("f2e59013a0a379837166b59f871b20a8a0d101d1c355ea85d35329360e69c000")
.unwrap(),
Expand All @@ -201,9 +225,42 @@ mod test {
);
}

#[test]
fn test_format_n_of_m_sub_metadata_in_the_wrong_order_builds_different_output() {
// We're passing the metadatas of 4 ISMs (indexes 0, 1, 2, 4) out of 5
let mut metadatas = vec![
SubModuleMetadata::new(
0,
Vec::from_hex("290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563")
.unwrap(),
),
SubModuleMetadata::new(
1,
Vec::from_hex("510e4e770828ddbf7f7b00ab00a9f6adaf81c0dc9cc85f1f8249c256942d61d9")
.unwrap(),
),
SubModuleMetadata::new(
4,
Vec::from_hex("f2e59013a0a379837166b59f871b20a8a0d101d1c355ea85d35329360e69c000")
.unwrap(),
),
SubModuleMetadata::new(
2,
Vec::from_hex("356e5a2cc1eba076e650ac7473fccc37952b46bc2e419a200cec0c451dce2336")
.unwrap(),
),
];
let expected = Vec::from_hex("000000280000004800000048000000680000006800000088000000000000000000000088000000a8290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563510e4e770828ddbf7f7b00ab00a9f6adaf81c0dc9cc85f1f8249c256942d61d9356e5a2cc1eba076e650ac7473fccc37952b46bc2e419a200cec0c451dce2336f2e59013a0a379837166b59f871b20a8a0d101d1c355ea85d35329360e69c000").unwrap();
assert_eq!(
AggregationIsmMetadataBuilder::format_metadata(&mut metadatas, 5),
expected
);
}


#[test]
fn test_format_empty_metadata_works_correctly() {
let mut metadatas = vec![SubIsmMetadata::new(0, Vec::from_hex("").unwrap())];
let mut metadatas = vec![SubModuleMetadata::new(0, Vec::from_hex("").unwrap())];
let expected = Vec::from_hex("0000000800000008").unwrap();
assert_eq!(
AggregationIsmMetadataBuilder::format_metadata(&mut metadatas, 1),
Expand All @@ -215,23 +272,23 @@ mod test {
fn test_n_cheapest_metas_works() {
let metas_and_gas = vec![
(
SubIsmMetadata::new(3, vec![]),
SubModuleMetadata::new(3, vec![]),
U256::from_dec_str("3").unwrap(),
),
(
SubIsmMetadata::new(2, vec![]),
SubModuleMetadata::new(2, vec![]),
U256::from_dec_str("2").unwrap(),
),
(
SubIsmMetadata::new(1, vec![]),
SubModuleMetadata::new(1, vec![]),
U256::from_dec_str("1").unwrap(),
),
];
assert_eq!(
AggregationIsmMetadataBuilder::n_cheapest_metas(metas_and_gas, 2),
vec![
SubIsmMetadata::new(1, vec![]),
SubIsmMetadata::new(2, vec![])
SubModuleMetadata::new(1, vec![]),
SubModuleMetadata::new(2, vec![])
]
)
}
Expand Down
2 changes: 2 additions & 0 deletions typescript/infra/config/environments/test/multisigIsm.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { ChainMap, ModuleType, MultisigIsmConfig } from '@hyperlane-xyz/sdk';

// the addresses here must line up with the e2e test's validator addresses
// Validators are anvil accounts 4-6
export const chainToValidator: Record<string, string> = {
test1: '0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65',
test2: '0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc',
Expand Down

0 comments on commit 022bd99

Please sign in to comment.