diff --git a/CHANGELOG.md b/CHANGELOG.md index 665d51f..fe6e2d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Rule evaluation errors are rate-limited: only the first occurrence and every 100th recurrence are logged (#60). - `SourceAccount`'s `allow`/`deny` fields are deprecated in favour of `mode` + `accounts`; both previously fired on list *membership*, which reversed the expected "alert on unknown caller" behaviour (#250). - `NoActivity` incidents now carry a unique `alert_id` per incident, and the recovery alert reuses its incident's `alert_id` so the two can be correlated (#254, #255). +- `alert_id` now hashes the network cursor ID, the rule type, the rule label and the rule's index within its contract's `rules` list — previously it only hashed `contract_id`, `tx_hash` and `rule_triggered`, so two rules with identical labels on one contract (e.g. two `LargeTransfer(>=100XLM)` rules routed to different webhooks) produced the same ID and were deduplicated away by receivers (#252). **IDs change**: receivers that persist `alert_id` for deduplication will see new values for alerts generated after upgrading, so old entries can no longer be matched against new ones. `EvalContext` gains a `network_id` field and `check_no_activity` gains a `rule_index` parameter. ### Removed diff --git a/README.md b/README.md index ed3982f..2683fcd 100644 --- a/README.md +++ b/README.md @@ -291,7 +291,7 @@ service. See [Multiple destinations](docs/configuration.md#multiple-destinations **Fields:** - `schema_version` — integer version of this payload shape (currently `1`). Additive changes (new optional fields) keep the same version; breaking changes (field removals or renames) bump it. Receivers should use this to detect incompatible changes. -- `alert_id` — stable, deterministic identifier derived from `(network, contract_id, tx_hash, rule_type, rule_triggered)` via SHA-256 prefix (32 hex chars). Identical for every retry of the same alert. Receivers should deduplicate on this value. +- `alert_id` — stable, deterministic identifier derived from `(network, contract_id, tx_hash, rule_type, rule_triggered, rule_index)` via SHA-256 prefix (32 hex chars), where `network` is the network cursor ID (e.g. `custom@https://…` for custom networks) and `rule_index` is the rule's position in the contract's `rules` list. Identical for every retry of the same alert, but never shared between two rules — even two rules with identical labels get different IDs (#252). Receivers should deduplicate on this value. - `rule_type` — stable machine-readable rule variant (e.g. `"LargeTransfer"`, `"HighFee"`); use this for programmatic routing - `rule_triggered` — human-readable rule description with parameters (e.g. `"LargeTransfer(>=10000XLM)"`); use this for display - `function_name` — the first invoked Soroban function name, or `null` for non-Soroban transactions. diff --git a/crates/notifier/src/lib.rs b/crates/notifier/src/lib.rs index 04564d3..3d68b79 100644 --- a/crates/notifier/src/lib.rs +++ b/crates/notifier/src/lib.rs @@ -368,12 +368,15 @@ pub fn test_payload_with_network( schema_version: 1, // Unique per send, so repeated tests are not de-duplicated by receivers. alert_id: txwatch_rules::alert_id( + network, TEST_CONTRACT_ID, tx_hash, + rule_type, &format!( "TestWebhook@{}", now.timestamp_nanos_opt().unwrap_or_default() ), + 0, ), label: label.to_string(), contract_id: TEST_CONTRACT_ID.into(), diff --git a/crates/poller/src/lib.rs b/crates/poller/src/lib.rs index b424dd8..4746f63 100644 --- a/crates/poller/src/lib.rs +++ b/crates/poller/src/lib.rs @@ -1535,6 +1535,7 @@ async fn poll_contract( label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: canonical_base, explorer_base: contract.network.explorer_base_url(), }; @@ -1779,6 +1780,7 @@ async fn poll_contract( } if let Some(payload) = check_no_activity( &rule.rule, + idx, state.last_seen, now, &mut state.no_activity_states[idx], @@ -2061,6 +2063,7 @@ fn evaluate_contract( label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base, explorer_base: contract.network.explorer_base_url(), }; diff --git a/crates/poller/tests/integration.rs b/crates/poller/tests/integration.rs index fc20f9d..ba433ec 100644 --- a/crates/poller/tests/integration.rs +++ b/crates/poller/tests/integration.rs @@ -272,6 +272,7 @@ async fn any_transaction_fires_webhook() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: &horizon.uri(), explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -377,6 +378,7 @@ async fn transaction_failed_rule_fires_only_on_failure() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: &horizon.uri(), explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -439,6 +441,7 @@ async fn large_transfer_fires_above_threshold() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }, @@ -527,6 +530,7 @@ async fn function_called_rule_fires_on_exact_match() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }, @@ -737,6 +741,7 @@ async fn high_fee_rule_fires_on_fee_charged() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: &horizon.uri(), explorer_base: Some("https://stellar.expert/explorer/testnet"), }, @@ -1757,6 +1762,7 @@ async fn large_transfer_only_counts_transfers_involving_the_contract() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -1834,6 +1840,7 @@ async fn large_transfer_respects_direction_option() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -1907,6 +1914,7 @@ async fn contract_upgraded_rule_fires_with_wasm_hashes() { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }, diff --git a/crates/rules/benches/evaluate.rs b/crates/rules/benches/evaluate.rs index 8c36a81..1d7e7db 100644 --- a/crates/rules/benches/evaluate.rs +++ b/crates/rules/benches/evaluate.rs @@ -67,6 +67,7 @@ fn bench_evaluate(c: &mut Criterion) { label: "bench", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; diff --git a/crates/rules/src/lib.rs b/crates/rules/src/lib.rs index 80b0dea..462f24c 100644 --- a/crates/rules/src/lib.rs +++ b/crates/rules/src/lib.rs @@ -541,8 +541,9 @@ pub struct AlertPayload { /// breaking changes (field removals or renames) bump it. pub schema_version: u32, /// Stable, deterministic alert identifier derived from - /// (network, contract_id, tx hash, rule_type, rule params) via SHA-256 - /// (issue #57). Receivers should deduplicate on this value. + /// (network cursor ID, contract_id, tx hash, rule_type, rule_triggered, + /// rule index) via SHA-256 (issues #57, #252). Receivers should + /// deduplicate on this value. /// Used as the PagerDuty `dedup_key`. #[serde(default)] pub alert_id: String, @@ -647,15 +648,37 @@ pub struct AlertPayload { } /// Deterministic alert ID: the first 32 hex characters (128 bits) of -/// SHA-256 over `contract_id`, `transaction_hash` and `rule_triggered`. -pub fn alert_id(contract_id: &str, transaction_hash: &str, rule_triggered: &str) -> String { +/// SHA-256 over the network cursor ID, `contract_id`, `transaction_hash`, +/// `rule_type`, `rule_triggered` and the rule's index within its contract's +/// rule list. +/// +/// Both the network and the rule identity are hashed (#252): two rules with +/// identical labels on one contract (e.g. two `LargeTransfer(>=100XLM)` rules +/// routed to different webhooks) must produce different IDs, otherwise +/// receivers deduplicate one of them away. `rule_index` is the position of the +/// rule in its contract's `rules` list, so it stays stable for a given config. +pub fn alert_id( + network: &str, + contract_id: &str, + transaction_hash: &str, + rule_type: &str, + rule_triggered: &str, + rule_index: usize, +) -> String { use sha2::{Digest, Sha256}; let mut hasher = Sha256::new(); - for part in [contract_id, transaction_hash, rule_triggered] { + for part in [ + network, + contract_id, + transaction_hash, + rule_type, + rule_triggered, + ] { hasher.update(part.as_bytes()); // Separator so ("ab", "c") and ("a", "bc") differ. hasher.update([0u8]); } + hasher.update(rule_index.to_le_bytes()); let mut id = hex::encode(hasher.finalize()); id.truncate(32); id @@ -705,6 +728,10 @@ pub struct EvalContext<'a> { pub contract_id: &'a str, /// Network the contract lives on (`"mainnet"`, `"testnet"`, …). pub network: &'a str, + /// Network cursor ID ([`txwatch_config::Network::cursor_id`]): the stable + /// per-network identity used in `alert_id` (#252). Unlike `network`, this + /// tells two custom networks apart (`"custom@https://…"`, see issue #252). + pub network_id: String, /// Horizon base URL used to build transaction links. pub horizon_base: &'a str, /// Explorer base URL for the network; `None` for custom networks that @@ -725,6 +752,7 @@ impl<'a> EvalContext<'a> { label: &contract.label, contract_id: &contract.contract_id, network: contract.network.as_str(), + network_id: contract.network.cursor_id(), horizon_base, explorer_base: contract.network.explorer_base_url(), } @@ -808,49 +836,61 @@ pub fn evaluate + RuleOverrides>( rules .iter() - .filter(|entry| entry.enabled()) - .filter_map(|entry| { + .enumerate() + .filter(|(_, entry)| entry.enabled()) + .filter_map(|(rule_index, entry)| { let rule: &AlertRule = entry.as_ref(); match eval_rule(rule, tx) { - Ok(true) => Some(AlertPayload { - schema_version: 1, - alert_id: alert_id(contract_id, &tx.hash, &rule.label()), - label: label.to_string(), - contract_id: contract_id.to_string(), - network: network.to_string(), - // #44: Use the single canonical implementations from txwatch-config. - rule_type: rule.rule_type().to_string(), - rule_triggered: rule.label(), - transaction_hash: tx.hash.clone(), - function_name: tx.function_names.first().cloned(), - function_names: tx.function_names.clone(), - function_args: tx.invocations.clone(), - amount_xlm, - amount_stroops: tx.amount_stroops, - amount_xlm_decimal: amount_xlm_decimal.clone(), - fee_charged_stroops: tx.fee_charged_stroops, - source_account: tx.source_account.clone(), - // Per-rule delivery overrides, absent unless the rule sets them. - severity: entry.severity().map(|s| s.to_string()), - effective_webhook_url: entry.webhook_url().map(str::to_owned), - effective_webhook_secret: entry.webhook_secret().map(str::to_owned), - timestamp, - timestamp_iso: timestamp_iso.clone(), - horizon_link: horizon_link.clone(), - explorer_link: explorer_link.clone(), - ledger: tx.ledger, - memo: tx.memo.clone(), - memo_type: tx.memo_type.clone(), - operation_count: tx.operation_count, - resolved: false, - matched_events: matched_events(rule, tx), - suppressed_count: 0, - observed_count: None, - test: false, - old_wasm_hash: upgrade_hashes(rule, tx).and_then(|(o, _)| o), - new_wasm_hash: upgrade_hashes(rule, tx).and_then(|(_, n)| n), - enrichment_incomplete: tx.enrichment_incomplete, - }), + Ok(true) => { + let rule_type = rule.rule_type().to_string(); + let rule_triggered = rule.label(); + Some(AlertPayload { + schema_version: 1, + alert_id: alert_id( + &ctx.network_id, + contract_id, + &tx.hash, + &rule_type, + &rule_triggered, + rule_index, + ), + label: label.to_string(), + contract_id: contract_id.to_string(), + network: network.to_string(), + // #44: Use the single canonical implementations from txwatch-config. + rule_type, + rule_triggered, + transaction_hash: tx.hash.clone(), + function_name: tx.function_names.first().cloned(), + function_names: tx.function_names.clone(), + function_args: tx.invocations.clone(), + amount_xlm, + amount_stroops: tx.amount_stroops, + amount_xlm_decimal: amount_xlm_decimal.clone(), + fee_charged_stroops: tx.fee_charged_stroops, + source_account: tx.source_account.clone(), + // Per-rule delivery overrides, absent unless the rule sets them. + severity: entry.severity().map(|s| s.to_string()), + effective_webhook_url: entry.webhook_url().map(str::to_owned), + effective_webhook_secret: entry.webhook_secret().map(str::to_owned), + timestamp, + timestamp_iso: timestamp_iso.clone(), + horizon_link: horizon_link.clone(), + explorer_link: explorer_link.clone(), + ledger: tx.ledger, + memo: tx.memo.clone(), + memo_type: tx.memo_type.clone(), + operation_count: tx.operation_count, + resolved: false, + matched_events: matched_events(rule, tx), + suppressed_count: 0, + observed_count: None, + test: false, + old_wasm_hash: upgrade_hashes(rule, tx).and_then(|(o, _)| o), + new_wasm_hash: upgrade_hashes(rule, tx).and_then(|(_, n)| n), + enrichment_incomplete: tx.enrichment_incomplete, + }) + } Ok(false) => None, Err(e) => { // A broken rule would otherwise warn on every poll cycle. @@ -1247,6 +1287,9 @@ pub enum NoActivityState { /// Check the `NoActivity` rule for a single contract after one poll cycle. /// /// * `rule` — must be `AlertRule::NoActivity { minutes }`. +/// * `rule_index` — position of this rule in the contract's `rules` +/// list, hashed into `alert_id` so two identical `NoActivity` rules on one +/// contract get distinct IDs (#252). /// * `last_seen` — the timestamp of the most recent transaction seen, /// or `None` if we have never seen any transaction. /// * `now` — current time (injectable for testing). @@ -1264,6 +1307,7 @@ pub enum NoActivityState { /// - Activity present and never breached → returns `None`. pub fn check_no_activity( rule: &AlertRule, + rule_index: usize, last_seen: Option>, now: DateTime, state: &mut NoActivityState, @@ -1298,7 +1342,14 @@ pub fn check_no_activity( // Threshold just exceeded for the first time → fire incident. (NoActivityState::Active, true) => { let rule_triggered = format!("NoActivity({}min)", minutes); - let incident_id = alert_id(ctx.contract_id, &ts.to_string(), &rule_triggered); + let incident_id = alert_id( + &ctx.network_id, + ctx.contract_id, + &ts.to_string(), + "NoActivity", + &rule_triggered, + rule_index, + ); *state = NoActivityState::Alerting { incident_alert_id: incident_id.clone(), }; @@ -1345,9 +1396,12 @@ pub fn check_no_activity( (NoActivityState::Alerting { incident_alert_id }, false) => { let recovery_alert_id = if incident_alert_id.is_empty() { alert_id( + &ctx.network_id, ctx.contract_id, &ts.to_string(), + "NoActivity", &format!("NoActivity({}min) resolved", minutes), + rule_index, ) } else { incident_alert_id.clone() @@ -1547,7 +1601,7 @@ impl CooldownTracker { let mut summaries = Vec::new(); - for rule_cfg in rules { + for (rule_index, rule_cfg) in rules.iter().enumerate() { if !rule_cfg.cooldown_summary { continue; } @@ -1573,7 +1627,14 @@ impl CooldownTracker { let ts = now.timestamp(); let ts_iso = now.format("%Y-%m-%dT%H:%M:%SZ").to_string(); - let alert_id = alert_id(ctx.contract_id, &ts.to_string(), &rule_trig); + let alert_id = alert_id( + &ctx.network_id, + ctx.contract_id, + &ts.to_string(), + rule_cfg.rule.rule_type(), + &rule_trig, + rule_index, + ); summaries.push(AlertPayload { label: ctx.label.to_string(), @@ -1825,6 +1886,7 @@ mod tests { label: "Label", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -1988,6 +2050,7 @@ mod tests { label: "L", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -2245,6 +2308,7 @@ mod tests { label: "L", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4", network: "testnet", + network_id: "testnet".into(), horizon_base, explorer_base, }; @@ -3088,6 +3152,7 @@ mod prop_tests { label: "PropTest", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -3167,6 +3232,7 @@ mod prop_tests { label: "PropTest", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -3190,6 +3256,7 @@ mod no_activity_tests { label: "Oracle", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), } @@ -3211,7 +3278,7 @@ mod no_activity_tests { // Last seen 6 minutes ago — threshold is 5 min, so this should fire. let last_seen = ts(12, 0); let now = ts(12, 6); - let payload = check_no_activity(&rule, Some(last_seen), now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, Some(last_seen), now, &mut state, &eval_ctx); assert!(payload.is_some(), "should fire when quiet for > threshold"); let p = payload.unwrap(); @@ -3239,7 +3306,7 @@ mod no_activity_tests { // Last seen 4 minutes ago — below threshold. let last_seen = ts(12, 0); let now = ts(12, 4); - let payload = check_no_activity(&rule, Some(last_seen), now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, Some(last_seen), now, &mut state, &eval_ctx); assert!(payload.is_none()); assert_eq!(state, NoActivityState::Active); @@ -3255,7 +3322,7 @@ mod no_activity_tests { // Exactly 5 minutes gap — should fire. let last_seen = ts(12, 0); let now = ts(12, 5); - let payload = check_no_activity(&rule, Some(last_seen), now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, Some(last_seen), now, &mut state, &eval_ctx); assert!(payload.is_some(), "should fire at exactly threshold"); } @@ -3272,7 +3339,7 @@ mod no_activity_tests { // Still quiet — already alerting, so no second payload. let last_seen = ts(12, 0); let now = ts(12, 20); - let payload = check_no_activity(&rule, Some(last_seen), now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, Some(last_seen), now, &mut state, &eval_ctx); assert!( payload.is_none(), @@ -3292,9 +3359,12 @@ mod no_activity_tests { let eval_ctx = contract; let rule = AlertRule::NoActivity { minutes: 5 }; let incident_id = alert_id( + &eval_ctx.network_id, eval_ctx.contract_id, &ts(12, 0).timestamp().to_string(), + "NoActivity", "NoActivity(5min)", + 0, ); let mut state = NoActivityState::Alerting { incident_alert_id: incident_id.clone(), @@ -3303,7 +3373,7 @@ mod no_activity_tests { // Activity just happened (1 minute ago) while we were in Alerting state. let last_seen = ts(12, 10); let now = ts(12, 11); - let payload = check_no_activity(&rule, Some(last_seen), now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, Some(last_seen), now, &mut state, &eval_ctx); assert!( payload.is_some(), @@ -3330,7 +3400,7 @@ mod no_activity_tests { let last_seen_1 = ts(12, 0); let now_1 = ts(12, 6); let payload_1 = - check_no_activity(&rule, Some(last_seen_1), now_1, &mut state, &eval_ctx).unwrap(); + check_no_activity(&rule, 0, Some(last_seen_1), now_1, &mut state, &eval_ctx).unwrap(); assert!(!payload_1.resolved); let incident_id_1 = payload_1.alert_id.clone(); assert_eq!( @@ -3344,7 +3414,7 @@ mod no_activity_tests { let last_seen_2 = ts(12, 10); let now_2 = ts(12, 11); let recovery_1 = - check_no_activity(&rule, Some(last_seen_2), now_2, &mut state, &eval_ctx).unwrap(); + check_no_activity(&rule, 0, Some(last_seen_2), now_2, &mut state, &eval_ctx).unwrap(); assert!(recovery_1.resolved); assert_eq!( recovery_1.alert_id, incident_id_1, @@ -3355,7 +3425,7 @@ mod no_activity_tests { // 3. Incident 2: quiet again at 12:20 let now_3 = ts(12, 20); let payload_2 = - check_no_activity(&rule, Some(last_seen_2), now_3, &mut state, &eval_ctx).unwrap(); + check_no_activity(&rule, 0, Some(last_seen_2), now_3, &mut state, &eval_ctx).unwrap(); assert!(!payload_2.resolved); let incident_id_2 = payload_2.alert_id.clone(); assert_eq!( @@ -3375,7 +3445,7 @@ mod no_activity_tests { let last_seen_3 = ts(12, 25); let now_4 = ts(12, 26); let recovery_2 = - check_no_activity(&rule, Some(last_seen_3), now_4, &mut state, &eval_ctx).unwrap(); + check_no_activity(&rule, 0, Some(last_seen_3), now_4, &mut state, &eval_ctx).unwrap(); assert!(recovery_2.resolved); assert_eq!( recovery_2.alert_id, incident_id_2, @@ -3393,7 +3463,7 @@ mod no_activity_tests { // No transactions ever seen — treated as infinite quiet period. let now = ts(12, 0); - let payload = check_no_activity(&rule, None, now, &mut state, &eval_ctx); + let payload = check_no_activity(&rule, 0, None, now, &mut state, &eval_ctx); assert!( payload.is_some(), @@ -3453,13 +3523,84 @@ mod suppressor_tests { #[test] fn alert_id_is_deterministic_and_distinguishes_inputs() { - let id = alert_id("CA", "tx1", "AnyTransaction"); - assert_eq!(id, alert_id("CA", "tx1", "AnyTransaction")); + let id = alert_id("testnet", "CA", "tx1", "AnyTransaction", "AnyTransaction", 0); + assert_eq!( + id, + alert_id("testnet", "CA", "tx1", "AnyTransaction", "AnyTransaction", 0) + ); assert_eq!(id.len(), 32); assert!(id.chars().all(|c| c.is_ascii_hexdigit())); - assert_ne!(id, alert_id("CA", "tx2", "AnyTransaction")); - assert_ne!(id, alert_id("CA", "tx1", "TransactionFailed")); - assert_ne!(alert_id("ab", "c", "r"), alert_id("a", "bc", "r")); + assert_ne!(id, alert_id("testnet", "CA", "tx2", "AnyTransaction", "AnyTransaction", 0)); + assert_ne!(id, alert_id("testnet", "CA", "tx1", "TransactionFailed", "TransactionFailed", 0)); + assert_ne!(id, alert_id("mainnet", "CA", "tx1", "AnyTransaction", "AnyTransaction", 0)); + assert_ne!(id, alert_id("testnet", "CA", "tx1", "LargeTransfer", "AnyTransaction", 0)); + assert_ne!(id, alert_id("testnet", "CA", "tx1", "AnyTransaction", "AnyTransaction", 1)); + assert_ne!( + alert_id("testnet", "ab", "c", "r", "r", 0), + alert_id("testnet", "a", "bc", "r", "r", 0) + ); + } + + /// Two rules with identical labels on one contract (e.g. two + /// `LargeTransfer(>=100XLM)` rules routed to different webhooks) must get + /// distinct `alert_id`s, otherwise receivers deduplicate one of them away + /// (#252). + #[test] + fn identical_rules_on_one_contract_do_not_collide() { + let tx = EnrichedTransaction { + hash: "deadbeef".into(), + timestamp: "2024-01-15T12:00:00Z".parse().unwrap(), + successful: true, + paging_token: "1".into(), + function_names: vec![], + amount_stroops: Some(500_000_000_000), + fee_charged_stroops: None, + source_account: None, + fee_account: None, + ledger: None, + memo: None, + memo_type: None, + operation_count: None, + watched_contract_id: "CAAA".into(), + events: vec![], + contract_transfers: vec![ContractTransfer { + from: "CAAA".into(), + to: "GBUYER".into(), + amount_stroops: 500_000_000_000, + }], + wasm_upgrade: None, + enrichment_incomplete: false, + }; + let ctx = EvalContext { + label: "L", + contract_id: "CAAA", + network: "testnet", + network_id: "testnet".into(), + horizon_base: "https://h", + explorer_base: Some("https://e"), + }; + let same_rule = || AlertRule::LargeTransfer { + threshold_xlm: 100, + direction: "any".into(), + threshold_stroops: 1_000_000_000, + }; + let payloads = evaluate(&ctx, &[same_rule(), same_rule()], &tx, None); + assert_eq!(payloads.len(), 2, "both duplicate rules must fire"); + assert_eq!(payloads[0].rule_triggered, payloads[1].rule_triggered); + assert_eq!(payloads[0].alert_id, alert_id( + "testnet", "CAAA", "deadbeef", "LargeTransfer", + &payloads[0].rule_triggered, 0, + )); + assert_ne!( + payloads[0].alert_id, payloads[1].alert_id, + "identical rules at different indexes must not share an alert_id" + ); + + // The same rule on a different network must not collide either. + let mut other = ctx.clone(); + other.network_id = "mainnet".into(); + let other_payloads = evaluate(&other, &[same_rule()], &tx, None); + assert_ne!(payloads[0].alert_id, other_payloads[0].alert_id); } #[test] @@ -3489,6 +3630,7 @@ mod suppressor_tests { label: "L", contract_id: "CAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://h", explorer_base: Some("https://e"), }; @@ -3501,7 +3643,14 @@ mod suppressor_tests { assert_eq!(payloads.len(), 2); assert_eq!( payloads[0].alert_id, - alert_id("CAAA", "deadbeef", "AnyTransaction") + alert_id( + "testnet", + "CAAA", + "deadbeef", + "AnyTransaction", + "AnyTransaction", + 0, + ) ); assert_ne!(payloads[0].alert_id, payloads[1].alert_id); } @@ -3634,6 +3783,7 @@ mod suppressor_tests { label: "Label", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABSC4", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), }; @@ -4091,6 +4241,7 @@ mod suppressor_tests { label: "Oracle", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), } diff --git a/crates/rules/tests/attack_tests.rs b/crates/rules/tests/attack_tests.rs index 65f6a80..b7d0dfd 100644 --- a/crates/rules/tests/attack_tests.rs +++ b/crates/rules/tests/attack_tests.rs @@ -33,6 +33,7 @@ fn test_ctx() -> EvalContext<'static> { label: "Escrow", contract_id: "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", network: "testnet", + network_id: "testnet".into(), horizon_base: "https://horizon-testnet.stellar.org", explorer_base: Some("https://stellar.expert/explorer/testnet"), } diff --git a/docs/alert-rules.md b/docs/alert-rules.md index b3186d3..5444518 100644 --- a/docs/alert-rules.md +++ b/docs/alert-rules.md @@ -576,9 +576,16 @@ All matching enabled rules fire; there is no short-circuit. ### `alert_id` and deduplication -`alert_id` is derived deterministically from `(network, contract_id, tx_hash, rule_type, rule_triggered)` -via SHA-256 (first 16 bytes → 32 hex chars). The same alert always produces the same `alert_id`, -so receivers can safely deduplicate retries and cursor replays by storing and checking this value. +`alert_id` is derived deterministically from +`(network cursor ID, contract_id, tx_hash, rule_type, rule_triggered, rule_index)` via SHA-256 +(first 16 bytes → 32 hex chars), where `rule_index` is the rule's position in its contract's +`rules` list. The same alert always produces the same `alert_id`, so receivers can safely +deduplicate retries and cursor replays by storing and checking this value. + +Because the network and the rule identity (type, label and index) are part of the hash, two rules +with identical labels on one contract — e.g. two `LargeTransfer(>=100XLM)` rules routed to +different webhooks — never collapse into the same `alert_id` (#252). + It is also sent as the `X-TxWatch-Alert-Id` request header, enabling deduplication without parsing the JSON body. diff --git a/docs/configuration.md b/docs/configuration.md index 886336c..105c9a9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -758,12 +758,12 @@ changes before parsing other fields. - `schema_version` — integer version of this payload shape. Use this to detect breaking changes. - `alert_id` — stable, deterministic identifier (32 hex chars) derived from - `(network, contract_id, tx_hash, rule_type, rule_triggered)` via SHA-256. Identical for every - retry of the same alert. Receivers should deduplicate on this value. Also sent as the - `X-TxWatch-Alert-Id` request header for deduplication without parsing the body. -- `alert_id` — stable 32-character hex ID derived from the contract, transaction and rule; the same - alert always has the same ID, so receivers can de-duplicate redeliveries. PagerDuty uses it as the - `dedup_key`. + `(network cursor ID, contract_id, tx_hash, rule_type, rule_triggered, rule_index)` via SHA-256, + where `rule_index` is the rule's position in its contract's `rules` list. Identical for every + retry of the same alert, but distinct for every rule — two rules with identical labels on one + contract never share an ID (#252). Receivers should deduplicate on this value; PagerDuty uses it + as the `dedup_key`. Also sent as the `X-TxWatch-Alert-Id` request header for deduplication + without parsing the body. - `rule_type` — stable machine-readable rule variant (e.g. `"LargeTransfer"`); use it for routing. - `rule_triggered` — human-readable rule description including parameters. - `amount_xlm` — whole-XLM transfer amount (truncated integer), or `null` when the transaction has none. Kept for backward compatibility — use `amount_xlm_decimal` for precise accounting.