Implement zeroShadow Hermod support#4426
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the Hermod (zeroShadow) sanctioned-address checker and refactors the banned-user detection logic into a trait-based system. The implementation includes HMAC-SHA256 address obfuscation, HTTP-based lookups, and a background cache maintenance task. Critical feedback was provided regarding the use of unbounded join_all for network requests; it is recommended to use concurrency-limited streams like buffer_unordered in both the lookup and maintenance flows to ensure system stability and avoid triggering rate limits.
| async fn check(&self, addresses: &HashSet<Address>, banned: &mut HashSet<Address>) { | ||
| let mut need_lookup = Vec::new(); | ||
| for address in addresses { | ||
| if banned.contains(address) { | ||
| continue; | ||
| } | ||
| match self.cache().get(address) { | ||
| Some(metadata) => { | ||
| metadata.is_banned.then(|| banned.insert(*address)); | ||
| } | ||
| None => need_lookup.push(*address), | ||
| } | ||
| } | ||
|
|
||
| let to_cache = join_all( | ||
| need_lookup | ||
| .into_iter() | ||
| .map(|address| async move { (address, self.fetch(address).await) }), | ||
| ) | ||
| .await; |
There was a problem hiding this comment.
Using join_all without a concurrency limit can lead to a burst of HTTP requests if need_lookup contains many addresses. This can cause stability issues or trigger rate limits on the backend. Consider using a concurrency-limited stream instead. Additionally, ensure that individual fetch errors (like timeouts) are handled explicitly and not cached, as per repository guidelines on batch operations and caching.
async fn check(&self, addresses: &HashSet<Address>, banned: &mut HashSet<Address>) {
use futures::StreamExt;
let mut need_lookup = Vec::new();
for address in addresses {
if banned.contains(address) {
continue;
}
match self.cache().get(address) {
Some(metadata) => {
metadata.is_banned.then(|| banned.insert(*address));
}
None => need_lookup.push(*address),
}
}
let to_cache = futures::stream::iter(need_lookup)
.map(|address| async move { (address, self.fetch(address).await) })
.buffer_unordered(10)
.collect::<Vec<_>>()
.await;
}References
- When fetching a batch of items where individual fetches can fail, do not silently ignore errors.
- Do not cache timeouts if the timeout is moved deeper into the code, as this might lead to incorrect caching of failures.
| let results = join_all( | ||
| expired_data | ||
| .into_iter() | ||
| .map(|(address, metadata)| detector.determine_status(*address, metadata)), | ||
| ) | ||
| .await | ||
| .into_iter() | ||
| .flatten(); |
There was a problem hiding this comment.
The background maintenance task uses join_all on all expired entries. If a large portion of the cache expires simultaneously, this will attempt to fire thousands of concurrent HTTP requests. Use buffer_unordered to limit concurrency. Additionally, ensure errors are not silently ignored (e.g., by using .flatten() on Results); in background tasks, unrecoverable errors should trigger a fail-fast response like panicking, as per repository guidelines.
let results = futures::stream::iter(expired_data)
.map(|(address, metadata)| detector.determine_status(*address, metadata))
.buffer_unordered(10)
.collect::<Vec<_>>()
.await
.into_iter()
.flatten();References
- In critical background tasks, panicking on unrecoverable errors is an acceptable strategy to ensure a fail-fast behavior.
- When fetching a batch of items where individual fetches can fail, do not silently ignore errors.
Description
Adds support for Hermod, zeroShadow's self-hosted sanctioned-address agent, as an additional banned-user backend alongside the existing Chainalysis on-chain oracle.
Hermod is queried over HTTP with an HMAC-SHA256 obfuscated form of the user address (per-customer key), so we never send a raw address off-host. A hit returns 200, a miss returns 404. Both backends can run together — an address is banned if either reports it as sanctioned.
Changes
HermodConfigincrates/configs/src/banned_users.rs(url,hmac_key, optionalapi_key), wired into bothautopilotandorderbookstartup.%ENV_VARindirection.deserialize_optional_string_from_envhelper for the optional API key.Debugimpl redacts both secrets.crates/order-validation/src/banned/hermod.rsimplementing the agent client: signs{address:#x}with HMAC-SHA256, hitsGET <base>/addresses/<hex-sig>, optionalBearerauth, base-URL trailing-slash normalisation.crates/order-validation/src/banned.rsto introduce aBackendtrait shared by the ChainalysisOnchainchecker and the newHermodchecker.Users::bannednow fans out to whichever subset of backends is configured.How to test
cargo nextest run -p configs -p order-validation— covers the new config parsing, redaction, env-var indirection, HMAC signing, and URL normalisation tests.