Relayer remote message-id denylist via AW registry#8340
Relayer remote message-id denylist via AW registry#8340
Conversation
nambrot
requested review from
Mo-Hussain,
Xaroz,
paulbalaji,
tkporter,
xeno097,
yjamin and
yorhodes
as code owners
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb6de3baf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if let Some(blacklist_url) = self.remote_blacklist_url.clone() { | ||
| let name = "remote_message_id_blacklist_updater"; | ||
| let message_blacklist = self.message_blacklist.clone(); | ||
| let inline_message_blacklist = self.inline_message_blacklist.clone(); | ||
| let blacklist_updater = tokio::task::Builder::new() |
There was a problem hiding this comment.
Relayer::run initializes message_blacklist from inline config and only spawns run_remote_blacklist_updater asynchronously here, then continues booting processors immediately. Because this commit removed the local customBlacklist.ts message-id entries, denylisted IDs now depend on that remote fetch; on restart, any queued denylisted message can be processed before the first HTTP refresh completes (or if the first refresh fails), which regresses blacklist enforcement at startup.
Useful? React with 👍 / 👎.
| #[serde(rename = "context")] | ||
| _context: String, |
There was a problem hiding this comment.
context in remote denylist entries
RemoteDenylistEntry makes context mandatory even though the updater only uses message_id; fetch_remote_message_ids deserializes the full struct, so any external payload entry missing context (or setting it null) causes the entire refresh to fail and keeps the previous blacklist. This creates avoidable fragility against schema drift in an external registry field the relayer does not consume.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| entries | ||
| .into_iter() | ||
| .map(|entry| { | ||
| entry.message_id.parse::<H256>().with_context(|| { | ||
| format!( | ||
| "Invalid messageId `{}` in remote blacklist payload", | ||
| entry.message_id | ||
| ) | ||
| }) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
🚩 Single malformed entry in remote JSON aborts entire fetch
In fetch_remote_message_ids at rust/main/agents/relayer/src/relayer.rs:975-985, the .collect::<Result<HashSet<H256>>>() short-circuits on the first H256 parse error, discarding all valid entries from that fetch cycle. The error is caught at line 946 and the previous blacklist is preserved. This fail-safe approach means a single malformed messageId in the remote JSON prevents any updates. Consider whether a .filter_map with per-entry logging would be more resilient, especially if the remote list is managed by multiple contributors.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
blacklistUrlconfig support end-to-end (TS config + Rust settings)customBlacklist.tsmessage-id list from infra configValidation
cargo fmt --package relayercargo check -p relayercargo test -p relayer test_fetch_remote_message_ids_ -- --nocapturecargo test -p relayer test_merge_message_blacklists_ -- --nocaptureSibling PRs / Context
main): hyperlane-xyz/aw-registry@9136bc0These three changes are intended to roll out together for ENG-3512.