diff --git a/crates/l1/src/state/tip403/cache.rs b/crates/l1/src/state/tip403/cache.rs index 142f61732..0c477c757 100644 --- a/crates/l1/src/state/tip403/cache.rs +++ b/crates/l1/src/state/tip403/cache.rs @@ -271,13 +271,13 @@ impl PolicyCacheInner { match policy_type { PolicyType::WHITELIST => { - if !policy.policy_set.is_known(&user) { + if !policy.policy_set.is_known(&user, block_number) { return None; } Some(policy.policy_set.contains(user, block_number)) } PolicyType::BLACKLIST => { - if !policy.policy_set.is_known(&user) { + if !policy.policy_set.is_known(&user, block_number) { return None; } Some(!policy.policy_set.contains(user, block_number)) @@ -323,13 +323,13 @@ impl PolicyCacheInner { match policy_type { PolicyType::WHITELIST => { - if !policy.policy_set.is_known(&user) { + if !policy.policy_set.is_known(&user, block_number) { return None; } Some(policy.policy_set.contains(user, block_number)) } PolicyType::BLACKLIST => { - if !policy.policy_set.is_known(&user) { + if !policy.policy_set.is_known(&user, block_number) { return None; } Some(!policy.policy_set.contains(user, block_number)) @@ -522,7 +522,7 @@ mod tests { // Stale writes must not mark unknown users as observed either. set.record_status(USER_B, 15, false); - assert!(!set.is_known(&USER_B)); + assert!(!set.is_known(&USER_B, 25)); set.record_status(USER_A, 21, false); assert!(!set.contains(USER_A, 21)); @@ -533,15 +533,15 @@ mod tests { let mut set = PolicySet::default(); set.record_status(USER_A, 0, false); - assert!(!set.is_known(&USER_A)); + assert!(!set.is_known(&USER_A, 5)); assert!(!set.contains(USER_A, 0)); set.record_status(USER_B, 0, true); - assert!(!set.is_known(&USER_B)); + assert!(!set.is_known(&USER_B, 5)); assert!(!set.contains(USER_B, 0)); set.record_status(USER_B, 1, true); - assert!(set.is_known(&USER_B)); + assert!(set.is_known(&USER_B, 1)); assert!(set.contains(USER_B, 1)); } @@ -1212,22 +1212,46 @@ mod tests { let mut set = PolicySet::default(); // Fresh set: nobody known - assert!(!set.is_known(&USER_A)); - assert!(!set.is_known(&USER_B)); + assert!(!set.is_known(&USER_A, 10)); + assert!(!set.is_known(&USER_B, 10)); // Add USER_A set.record_status(USER_A, 10, true); - assert!(set.is_known(&USER_A)); - assert!(!set.is_known(&USER_B)); + assert!(set.is_known(&USER_A, 10)); + assert!(!set.is_known(&USER_B, 10)); + + // A query before USER_A's first observed event is not authoritative. + assert!(!set.is_known(&USER_A, 9)); // Advance past the event set.advance(20); - assert!(set.is_known(&USER_A), "observed must survive advance"); - assert!(!set.is_known(&USER_B)); + assert!( + set.is_known(&USER_A, 20), + "first_seen must survive advance" + ); + assert!(!set.is_known(&USER_B, 20)); // Clear resets everything set.clear(); - assert!(!set.is_known(&USER_A)); + assert!(!set.is_known(&USER_A, 10)); + } + + #[test] + fn policy_set_future_delta_does_not_authorize_earlier_query() { + // Regression: the subscriber runs ahead of the engine, so the set can hold a + // user's future delta while the engine queries an earlier block. The only event + // we have for USER_A is a removal at block 150; that tells us nothing about the + // user's membership at block 120, so an earlier query must be reported as unknown + // (forcing RPC fallback) rather than authoritatively "absent". + let mut set = PolicySet::default(); + set.record_status(USER_A, 150, false); + + assert!( + !set.is_known(&USER_A, 120), + "a future delta must not make an earlier block authoritative" + ); + assert!(set.is_known(&USER_A, 150)); + assert!(set.is_known(&USER_A, 200)); } #[test] diff --git a/crates/l1/src/state/tip403/policy_set.rs b/crates/l1/src/state/tip403/policy_set.rs index 2033829bf..f75d922a6 100644 --- a/crates/l1/src/state/tip403/policy_set.rs +++ b/crates/l1/src/state/tip403/policy_set.rs @@ -1,7 +1,7 @@ //! Block-versioned policy sets for TIP-403 policy tracking. use alloy_primitives::Address; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; /// Block-versioned policy set for TIP-403 policy tracking. /// @@ -19,9 +19,11 @@ pub struct PolicySet { baseline_height: u64, /// Per-block set updates above `baseline_height`. pending: BTreeMap>, - /// All addresses for which we've ever recorded a set event. Survives `advance()` so - /// we can distinguish "explicitly absent from the set" from "never observed by the subscriber". - observed: HashSet
, + /// Earliest block at which we recorded a set event for each address. Survives `advance()` + /// so we can distinguish "explicitly absent from the set" from "never observed by the + /// subscriber", *and* so that known-ness is scoped to the queried block: an event only + /// tells us the membership at and after the block it was observed on. + first_seen: HashMap, } impl PolicySet { @@ -45,12 +47,23 @@ impl PolicySet { self.baseline.contains(&user) } - /// Returns `true` if we've ever recorded a set event for `user` (added or removed). + /// Returns `true` if [`contains`](Self::contains) can be trusted for `user` at + /// `block_number`. /// - /// When `false`, the caller should not trust [`contains`](Self::contains) returning `false` - /// because the user may have been added before the subscriber started. - pub fn is_known(&self, user: &Address) -> bool { - self.observed.contains(user) || self.baseline.contains(user) + /// A set event only reveals membership at and after the block it was observed on. We can + /// therefore only answer authoritatively for a query block that is at or after the earliest + /// event we recorded for the user; below that block (or with no event at all) the user may + /// have been added before we started observing, so the caller must fall back to RPC. + /// + /// This scoping matters because the subscriber runs ahead of the engine: the set can hold a + /// user's *future* delta (e.g. a block-150 removal) while the engine queries an earlier block + /// (e.g. block 120). Without the block bound, that future delta would mark the user "known" + /// and suppress the RPC fallback, so a blacklist membership that only exists before the delta + /// would be silently treated as absent. + pub fn is_known(&self, user: &Address, block_number: u64) -> bool { + self.first_seen + .get(user) + .is_some_and(|&first| first <= block_number) } /// Record a set update at the given block height. @@ -63,7 +76,10 @@ impl PolicySet { return; } - self.observed.insert(user); + self.first_seen + .entry(user) + .and_modify(|first| *first = (*first).min(block_number)) + .or_insert(block_number); self.pending .entry(block_number) .or_default() @@ -110,7 +126,7 @@ impl PolicySet { self.baseline.clear(); self.baseline_height = 0; self.pending.clear(); - self.observed.clear(); + self.first_seen.clear(); } }