Skip to content

Commit 8af3e37

Browse files
danwtclaude
andcommitted
claude: address second round of review feedback
BLOCKING fixes: - Fix warn! to error! for failed delivery tx storage - Remove success/debug logging for delivery tx storage (follow log-after-action rule) - Remove leftover debug warn! with ALL CAPS in hyperlane_db.rs - Remove CORS from /reprocess_message (security: POST endpoints should not have permissive CORS) - Remove unused origin_db_delivery field from MessageContext MAJOR fixes: - Remove duplicate tx->message_id reverse mapping from delivery_db.rs (origin-side mapping via HyperlaneDb is sufficient for /dispatched endpoint) - Remove logging from low-level DB methods (let callers decide) Simplifies delivery_db.rs to only store message_id -> delivery_tx mapping, which is what the /delivered endpoint actually uses. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d992989 commit 8af3e37

6 files changed

Lines changed: 7 additions & 59 deletions

File tree

rust/main/agents/relayer/src/msg/pending_message.rs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ pub struct MessageContext {
6868
pub destination_mailbox: Arc<dyn Mailbox>,
6969
/// Origin chain database to verify gas payments.
7070
pub origin_db: Arc<dyn HyperlaneDb>,
71-
/// Origin chain database for reverse lookup (tx_hash -> message_id).
72-
pub origin_db_delivery: Arc<dyn DeliveryDb>,
73-
/// Destination chain database for storing delivery info.
71+
/// Destination chain database for storing delivery tx hashes.
7472
pub destination_db: Arc<dyn DeliveryDb>,
7573
/// Cache to store commonly used data calls.
7674
pub cache: OptionalCache<MeteredCache<LocalCache>>,
@@ -933,27 +931,14 @@ impl PendingMessage {
933931
.destination_db
934932
.store_delivery_tx(&message_id, &outcome.transaction_id)
935933
{
936-
warn!(
934+
error!(
937935
message_id = ?message_id,
938936
tx_id = ?outcome.transaction_id,
939937
destination = ?self.message.destination,
940938
error = %e,
941-
"DELIVERY_STORAGE: Failed to store delivery tx hash on destination"
942-
);
943-
} else {
944-
warn!(
945-
message_id = ?message_id,
946-
tx_id = ?outcome.transaction_id,
947-
destination = ?self.message.destination,
948-
"DELIVERY_STORAGE: Successfully stored delivery tx hash on destination"
939+
"failed to store delivery tx hash"
949940
);
950941
}
951-
} else {
952-
warn!(
953-
message_id = ?self.message.id(),
954-
destination = ?self.message.destination,
955-
"DELIVERY_STORAGE: No submission_outcome available, cannot store delivery tx hash"
956-
);
957942
}
958943

959944
Ok(())

rust/main/agents/relayer/src/relayer.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,9 @@ impl BaseAgent for Relayer {
256256
origin_chain_setup.ignore_reorg_reports,
257257
);
258258

259-
// Create destination_db for storing delivery info
259+
// Create destination_db for storing delivery tx hashes
260260
let destination_db: Arc<dyn DeliveryDb> =
261261
Arc::new(destination.database.clone());
262-
263-
// Create origin_db_delivery for reverse lookup (tx_hash -> message_id)
264-
let origin_db_delivery: Arc<dyn DeliveryDb> =
265-
Arc::new(db.clone());
266262

267263
msg_ctxs.insert(
268264
ContextKey {
@@ -272,7 +268,6 @@ impl BaseAgent for Relayer {
272268
Arc::new(MessageContext {
273269
destination_mailbox: destination_mailbox.clone(),
274270
origin_db: Arc::new(db.clone()),
275-
origin_db_delivery,
276271
destination_db,
277272
cache: cache.clone(),
278273
metadata_builder: Arc::new(metadata_builder),

rust/main/agents/relayer/src/server/operations/reprocess_message.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::{cmp::Reverse, collections::HashMap, str::FromStr, sync::Arc};
22

33
use axum::{extract::State, http::StatusCode, routing, Json, Router};
44
use derive_new::new;
5-
use tower_http::cors::{Any, CorsLayer};
65
use hyperlane_base::{
76
db::{HyperlaneDb, HyperlaneRocksDB},
87
server::utils::{ServerErrorBody, ServerErrorResponse, ServerResult, ServerSuccessResponse},
@@ -24,14 +23,8 @@ pub struct ServerState {
2423

2524
impl ServerState {
2625
pub fn router(self) -> Router {
27-
let cors = CorsLayer::new()
28-
.allow_origin(Any)
29-
.allow_methods(Any)
30-
.allow_headers(Any);
31-
3226
Router::new()
3327
.route("/reprocess_message", routing::post(handler))
34-
.layer(cors)
3528
.with_state(self)
3629
}
3730
}

rust/main/agents/relayer/src/test_utils/dummy_data.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ pub fn dummy_message_context(
118118
MessageContext {
119119
destination_mailbox: Arc::new(MockMailboxContract::new_with_default_ism(H256::zero())),
120120
origin_db: Arc::new(db.clone()),
121-
origin_db_delivery: Arc::new(db.clone()),
122121
destination_db: Arc::new(db.clone()),
123122
cache,
124123
metadata_builder: base_metadata_builder,
Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,21 @@
11
use eyre::Result;
2-
use tracing::{debug, error};
32

43
use hyperlane_core::{Encode, H256, H512};
54

65
use super::HyperlaneRocksDB;
76

87
const DELIVERY_TX: &str = "delivery_tx_";
9-
const MESSAGE_ID_BY_TX: &str = "message_id_by_tx_";
108

119
/// Implementation of DeliveryDb for HyperlaneRocksDB
12-
/// Tracks delivery transaction hashes for all destination chains
10+
/// Tracks delivery transaction hashes for destination chains
1311
impl hyperlane_core::DeliveryDb for HyperlaneRocksDB {
1412
fn store_delivery_tx(&self, message_id: &H256, destination_tx: &H512) -> Result<()> {
1513
self.store_encodable(DELIVERY_TX, message_id.to_vec(), destination_tx)?;
16-
self.store_encodable(MESSAGE_ID_BY_TX, destination_tx.to_vec(), message_id)?;
17-
debug!(message_id = ?message_id, destination_tx = ?destination_tx, "stored delivery tx");
1814
Ok(())
1915
}
2016

2117
fn retrieve_delivery_tx(&self, message_id: &H256) -> Result<Option<H512>> {
22-
match self.retrieve_decodable(DELIVERY_TX, message_id.to_vec()) {
23-
Ok(Some(tx)) => {
24-
debug!(message_id = ?message_id, tx_hash = ?tx, "found delivery tx");
25-
Ok(Some(tx))
26-
}
27-
Ok(None) => {
28-
debug!(message_id = ?message_id, "no delivery tx found");
29-
Ok(None)
30-
}
31-
Err(e) => {
32-
error!(message_id = ?message_id, error = %e, "error retrieving delivery tx");
33-
Err(e.into())
34-
}
35-
}
18+
self.retrieve_decodable(DELIVERY_TX, message_id.to_vec())
19+
.map_err(Into::into)
3620
}
3721
}

rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,6 @@ impl HyperlaneRocksDB {
8989
dispatch_tx_id: &H512,
9090
) -> DbResult<bool> {
9191
self.store_message(message, dispatched_block_number)?;
92-
// - `dispatch_tx_id` --> `message_id` (for /delivered/by_tx API)
93-
warn!(
94-
dispatch_tx_id = ?dispatch_tx_id,
95-
message_id = ?message.id(),
96-
origin_domain = %message.origin,
97-
destination_domain = %message.destination,
98-
"STORING MESSAGE ID BY DISPATCH TX"
99-
);
10092
self.store_message_id_by_dispatch_tx(dispatch_tx_id, &message.id())?;
10193
Ok(true)
10294
}

0 commit comments

Comments
 (0)