Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(minor-router)!: change u32 to u64 in message ids #666

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Changelog

## [Unreleased](https://github.com/axelarnetwork/axelar-amplifier/tree/HEAD)

[Full Changelog](https://github.com/axelarnetwork/axelar-amplifier/compare/ampd-v1.2.0..HEAD)

- Change event index in message ids from u32 to u64. Emit message id from voting verifier [#666](https://github.com/axelarnetwork/axelar-amplifier/pull/666)

#### Migration Notes

The voting verifier contracts must be migrated before ampd is upgraded.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The voting verifier contracts must be migrated before ampd is upgraded.
The voting verifier contracts must be migrated before ampd is upgraded. Existing ampd instances will continue to work even after the contract migration, but we recommend upgrading it.

15 changes: 8 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 28 additions & 21 deletions ampd/src/evm/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use axelar_wasm_std::voting::Vote;
use ethers_contract::EthLogDecode;
use ethers_core::types::{Log, TransactionReceipt, H256};
use evm_gateway::{IAxelarAmplifierGatewayEvents, WeightedSigners};
use num_traits::cast;

use crate::handlers::evm_verify_msg::Message;
use crate::handlers::evm_verify_verifier_set::VerifierSetConfirmation;
Expand All @@ -16,7 +15,7 @@ impl PartialEq<IAxelarGatewayEventsWithLog<'_>> for &Message {

match event {
IAxelarAmplifierGatewayEvents::ContractCallFilter(event) => {
log.transaction_hash == Some(self.tx_id)
log.transaction_hash == Some(self.message_id.tx_hash.into())
&& event.sender == self.source_address
&& self.destination_chain == event.destination_chain
&& event.destination_contract_address == self.destination_address
Expand All @@ -37,7 +36,7 @@ impl PartialEq<IAxelarGatewayEventsWithLog<'_>> for &VerifierSetConfirmation {
Err(_) => return false,
};

log.transaction_hash == Some(self.tx_id)
log.transaction_hash == Some(self.message_id.tx_hash.into())
&& event.signers_hash == weighted_signers.hash()
&& event.signers == weighted_signers.abi_encode()
}
Expand All @@ -53,9 +52,9 @@ fn has_failed(tx_receipt: &TransactionReceipt) -> bool {
fn event<'a>(
gateway_address: &EVMAddress,
tx_receipt: &'a TransactionReceipt,
log_index: u32,
log_index: u64,
) -> Option<IAxelarGatewayEventsWithLog<'a>> {
let log_index: usize = cast(log_index).expect("log_index must be a valid usize");
let log_index: usize = usize::try_from(log_index).ok()?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add test case to cover this failure?


tx_receipt
.logs
Expand All @@ -74,7 +73,7 @@ fn verify<'a, V>(
tx_receipt: &'a TransactionReceipt,
to_verify: V,
expected_transaction_hash: H256,
expected_event_index: u32,
expected_event_index: u64,
) -> Vote
where
V: PartialEq<IAxelarGatewayEventsWithLog<'a>>,
Expand All @@ -98,7 +97,13 @@ pub fn verify_message(
tx_receipt: &TransactionReceipt,
msg: &Message,
) -> Vote {
verify(gateway_address, tx_receipt, msg, msg.tx_id, msg.event_index)
verify(
gateway_address,
tx_receipt,
msg,
msg.message_id.tx_hash.into(),
msg.message_id.event_index,
)
}

pub fn verify_verifier_set(
Expand All @@ -110,13 +115,14 @@ pub fn verify_verifier_set(
gateway_address,
tx_receipt,
confirmation,
confirmation.tx_id,
confirmation.event_index,
confirmation.message_id.tx_hash.into(),
confirmation.message_id.event_index,
)
}

#[cfg(test)]
mod tests {
use axelar_wasm_std::msg_id::HexTxHashAndEventIndex;
use axelar_wasm_std::voting::Vote;
use cosmwasm_std::Uint128;
use ethers_contract::EthEvent;
Expand All @@ -137,7 +143,7 @@ mod tests {
let (gateway_address, tx_receipt, mut verifier_set) =
matching_verifier_set_and_tx_receipt();

verifier_set.tx_id = Hash::random();
verifier_set.message_id.tx_hash = Hash::random().into();
assert_eq!(
verify_verifier_set(&gateway_address, &tx_receipt, &verifier_set),
Vote::NotFound
Expand Down Expand Up @@ -172,17 +178,17 @@ mod tests {
let (gateway_address, tx_receipt, mut verifier_set) =
matching_verifier_set_and_tx_receipt();

verifier_set.event_index = 0;
verifier_set.message_id.event_index = 0;
assert_eq!(
verify_verifier_set(&gateway_address, &tx_receipt, &verifier_set),
Vote::NotFound
);
verifier_set.event_index = 2;
verifier_set.message_id.event_index = 2;
assert_eq!(
verify_verifier_set(&gateway_address, &tx_receipt, &verifier_set),
Vote::NotFound
);
verifier_set.event_index = 3;
verifier_set.message_id.event_index = 3;
assert_eq!(
verify_verifier_set(&gateway_address, &tx_receipt, &verifier_set),
Vote::NotFound
Expand Down Expand Up @@ -215,7 +221,7 @@ mod tests {
fn should_not_verify_msg_if_tx_id_does_not_match() {
let (gateway_address, tx_receipt, mut msg) = matching_msg_and_tx_receipt();

msg.tx_id = Hash::random();
msg.message_id.tx_hash = Hash::random().into();
assert_eq!(
verify_message(&gateway_address, &tx_receipt, &msg),
Vote::NotFound
Expand Down Expand Up @@ -248,17 +254,17 @@ mod tests {
fn should_not_verify_msg_if_log_index_does_not_match() {
let (gateway_address, tx_receipt, mut msg) = matching_msg_and_tx_receipt();

msg.event_index = 0;
msg.message_id.event_index = 0;
assert_eq!(
verify_message(&gateway_address, &tx_receipt, &msg),
Vote::NotFound
);
msg.event_index = 2;
msg.message_id.event_index = 2;
assert_eq!(
verify_message(&gateway_address, &tx_receipt, &msg),
Vote::NotFound
);
msg.event_index = 3;
msg.message_id.event_index = 3;
assert_eq!(
verify_message(&gateway_address, &tx_receipt, &msg),
Vote::NotFound
Expand Down Expand Up @@ -295,8 +301,7 @@ mod tests {
let verifier_set = build_verifier_set(KeyType::Ecdsa, &ecdsa_test_data::signers());

let verifier_set = VerifierSetConfirmation {
tx_id,
event_index: log_index,
message_id: HexTxHashAndEventIndex::new(tx_id, log_index as u64),
verifier_set,
};

Expand Down Expand Up @@ -331,8 +336,10 @@ mod tests {
let gateway_address = EVMAddress::random();

let msg = Message {
tx_id,
event_index: log_index,
message_id: HexTxHashAndEventIndex::new(tx_id, log_index as u64)
.to_string()
.parse()
.unwrap(),
source_address: "0xd48e199950589a4336e4dc43bd2c72ba0c0baa86"
.parse()
.unwrap(),
Expand Down
44 changes: 26 additions & 18 deletions ampd/src/handlers/evm_verify_msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@

#[derive(Deserialize, Debug)]
pub struct Message {
pub tx_id: Hash,
pub event_index: u32,
pub message_id: HexTxHashAndEventIndex,
pub destination_address: String,
pub destination_chain: ChainName,
pub source_address: EVMAddress,
Expand Down Expand Up @@ -121,7 +120,6 @@
})
.collect())
}

fn vote_msg(&self, poll_id: PollId, votes: Vec<Vote>) -> MsgExecuteContract {
MsgExecuteContract {
sender: self.verifier.as_ref().clone(),
Expand Down Expand Up @@ -174,24 +172,25 @@
return Ok(vec![]);
}

let tx_hashes: HashSet<_> = messages.iter().map(|message| message.tx_id).collect();
let tx_hashes: HashSet<Hash> = messages
.iter()
.map(|msg| msg.message_id.tx_hash.into())
.collect();
let finalized_tx_receipts = self
.finalized_tx_receipts(tx_hashes, confirmation_height)
.await?;

let poll_id_str: String = poll_id.into();
let source_chain_str: String = source_chain.into();
let message_ids = messages
.iter()
.map(|message| {
HexTxHashAndEventIndex::new(message.tx_id, message.event_index).to_string()
})
.collect::<Vec<_>>();
let votes = info_span!(
"verify messages from an EVM chain",
poll_id = poll_id_str,
source_chain = source_chain_str,
message_ids = message_ids.as_value()
message_ids = messages
.iter()
.map(|msg| msg.message_id.to_string())
.collect::<Vec<String>>()
.as_value(),

Check warning on line 193 in ampd/src/handlers/evm_verify_msg.rs

View check run for this annotation

Codecov / codecov/patch

ampd/src/handlers/evm_verify_msg.rs#L190-L193

Added lines #L190 - L193 were not covered by tests
)
.in_scope(|| {
info!("ready to verify messages in poll",);
Expand All @@ -200,7 +199,7 @@
.iter()
.map(|msg| {
finalized_tx_receipts
.get(&msg.tx_id)
.get(&msg.message_id.tx_hash.into())

Check warning on line 202 in ampd/src/handlers/evm_verify_msg.rs

View check run for this annotation

Codecov / codecov/patch

ampd/src/handlers/evm_verify_msg.rs#L202

Added line #L202 was not covered by tests
.map_or(Vote::NotFound, |tx_receipt| {
verify_message(&source_gateway_address, tx_receipt, msg)
})
Expand All @@ -226,6 +225,7 @@
use std::convert::TryInto;
use std::str::FromStr;

use axelar_wasm_std::msg_id::HexTxHashAndEventIndex;
use base64::engine::general_purpose::STANDARD;
use base64::Engine;
use cosmwasm_std;
Expand All @@ -247,6 +247,11 @@
use crate::PREFIX;

fn poll_started_event(participants: Vec<TMAddress>, expires_at: u64) -> PollStarted {
let msg_ids = vec![
HexTxHashAndEventIndex::new(Hash::random(), 0u64),
HexTxHashAndEventIndex::new(Hash::random(), 1u64),
HexTxHashAndEventIndex::new(Hash::random(), 10u64),
];
PollStarted::Messages {
metadata: PollMetadata {
poll_id: "100".parse().unwrap(),
Expand All @@ -263,24 +268,27 @@
},
messages: vec![
TxEventConfirmation {
tx_id: format!("0x{:x}", Hash::random()).parse().unwrap(),
event_index: 0,
tx_id: msg_ids[0].tx_hash_as_hex(),

Check warning on line 271 in ampd/src/handlers/evm_verify_msg.rs

View workflow job for this annotation

GitHub Actions / Test Suite

use of deprecated field `voting_verifier::events::TxEventConfirmation::tx_id`: use message_id field instead
event_index: msg_ids[0].event_index as u32,

Check warning on line 272 in ampd/src/handlers/evm_verify_msg.rs

View workflow job for this annotation

GitHub Actions / Test Suite

use of deprecated field `voting_verifier::events::TxEventConfirmation::event_index`: use message_id field instead
message_id: msg_ids[0].to_string().parse().unwrap(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll get deprecated warnings on tests now, can we ignore these warnings here and add a TODO to remove them?

source_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
destination_chain: "ethereum".parse().unwrap(),
destination_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
payload_hash: Hash::random().to_fixed_bytes(),
},
TxEventConfirmation {
tx_id: format!("0x{:x}", Hash::random()).parse().unwrap(),
event_index: 1,
tx_id: msg_ids[1].tx_hash_as_hex(),

Check warning on line 280 in ampd/src/handlers/evm_verify_msg.rs

View workflow job for this annotation

GitHub Actions / Test Suite

use of deprecated field `voting_verifier::events::TxEventConfirmation::tx_id`: use message_id field instead
event_index: msg_ids[1].event_index as u32,
message_id: msg_ids[1].to_string().parse().unwrap(),
source_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
destination_chain: "ethereum".parse().unwrap(),
destination_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
payload_hash: Hash::random().to_fixed_bytes(),
},
TxEventConfirmation {
tx_id: format!("0x{:x}", Hash::random()).parse().unwrap(),
event_index: 10,
tx_id: msg_ids[2].tx_hash_as_hex(),
event_index: msg_ids[2].event_index as u32,
message_id: msg_ids[2].to_string().parse().unwrap(),
source_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
destination_chain: "ethereum".parse().unwrap(),
destination_address: format!("0x{:x}", EVMAddress::random()).parse().unwrap(),
Expand Down
15 changes: 8 additions & 7 deletions ampd/src/handlers/evm_verify_verifier_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@

#[derive(Deserialize, Debug)]
pub struct VerifierSetConfirmation {
pub tx_id: Hash,
pub event_index: u32,
pub message_id: HexTxHashAndEventIndex,
pub verifier_set: VerifierSet,
}

Expand Down Expand Up @@ -166,14 +165,13 @@
}

let tx_receipt = self
.finalized_tx_receipt(verifier_set.tx_id, confirmation_height)
.finalized_tx_receipt(verifier_set.message_id.tx_hash.into(), confirmation_height)
.await?;
let vote = info_span!(
"verify a new verifier set for an EVM chain",
poll_id = poll_id.to_string(),
source_chain = source_chain.to_string(),
id = HexTxHashAndEventIndex::new(verifier_set.tx_id, verifier_set.event_index)
.to_string()
id = verifier_set.message_id.to_string()

Check warning on line 174 in ampd/src/handlers/evm_verify_verifier_set.rs

View check run for this annotation

Codecov / codecov/patch

ampd/src/handlers/evm_verify_verifier_set.rs#L174

Added line #L174 was not covered by tests
)
.in_scope(|| {
info!("ready to verify a new verifier set in poll");
Expand Down Expand Up @@ -201,6 +199,7 @@
use std::convert::TryInto;
use std::str::FromStr;

use axelar_wasm_std::msg_id::HexTxHashAndEventIndex;
use base64::engine::general_purpose::STANDARD;
use base64::Engine;
use error_stack::{Report, Result};
Expand Down Expand Up @@ -271,10 +270,12 @@
}

fn poll_started_event(participants: Vec<TMAddress>, expires_at: u64) -> PollStarted {
let msg_id = HexTxHashAndEventIndex::new(Hash::random(), 100u64);
PollStarted::VerifierSet {
verifier_set: VerifierSetConfirmation {
tx_id: format!("0x{:x}", Hash::random()).parse().unwrap(),
event_index: 100,
tx_id: msg_id.tx_hash_as_hex(),
event_index: msg_id.event_index as u32,
message_id: msg_id.to_string().parse().unwrap(),
verifier_set: build_verifier_set(KeyType::Ecdsa, &ecdsa_test_data::signers()),
},
metadata: PollMetadata {
Expand Down
Loading
Loading