Migrate reactive test library to Reactive Omni

[daniil-parsiq]

Summary

Re-implements this Foundry test library to simulate the Reactive Omni (CometBFT / shared-EVM)
network instead of the legacy (Geth + Prysm / per-user-RVM) network, per
specs/001-omni-migration.md. No backward compatibility with the legacy API is kept.

Instead of re-mocking behavior, the library now depends on the real Omni system contracts
(vendored as submodules) and deploys them into the test EVM:

• the SystemContract etched at 0x8888…8888 behind an ERC1967 proxy;
• a fresh CallbackProxy per registered non-reactive chain;
• a log-derived subscription index (a Solidity port of the node's MemFilterService);
• a faithful match → trigger → callback pipeline that replays the recorded log stream.

87 tests pass; forge fmt --check / build --sizes / test are all green under the ci profile.

What changed

New src/ (replaces all legacy mock/, simulator/, base/, constants/, interfaces/):

File	Role
OmniConstants.sol	addresses, event/cron topic hashes (all cast keccak-verified), gas params
ReactiveTypes.sol	FeeMode, Subscription, CallbackResult, PendingCallback, DeferredSub, ReactiveEnv
SubscriptionIndex.sol	MemFilterService port — positional matching, dedup, idempotent add, order-preserving remove, blacklist/whitelist
ReactEngine.sol	the §7 pipeline (trigger, CallbackRequest capture + sender injection, proxy delivery, fee modes, deferred §7.1 changes, cron expansion)
ReactiveTest.sol	public base: _reactiveSetUp, _registerChain (+ custom-senders overload), _getCallbackProxy, _registerContract, _switchToChain, _react(FeeMode), _reactUntilQuiescent, _emitCron, _fund, and the _recordedLogs/_assertEmitted/… capture helpers
env/EnvironmentDeployer.sol	etches the system contract; deploys callback proxies

Tests — rewritten as 11 scenario/unit suites (BasicDemo, CallbackAuth, SelfCallback,
SubscriptionFiltering, MultiStep, ChainRegistry, CronDemo, plus Environment,
EnvironmentGuards, SubscriptionIndex, ReactEngine) on 6 omni-style mocks.

Config — foundry.toml (solc 0.8.29 + cancun + via_ir + ci profile + fmt/lint), remappings.txt,
two submodules (reactive-lib-omni, system-smart-contracts@lasna2-dev), rewritten README.md.

Decisions taken beyond the initial spec

The spec's §12 already resolves cron synthesis, dropping reactive-chain callbacks, the callback-sender
identity, FeeMode, dropping the legacy Callback event, deferred subscriptions, and solc/evm. The
following decisions were not in the spec and were made during implementation:

1. via_ir = true + optimizer are required (and leak to consumers). The spec mandated solc 0.8.29

   • cancun but not via_ir. The ReactEngine pipeline is deep enough that its internal library
     code hits "stack too deep" without IR. Because that code inlines into the user's test contract,
     consumers must also set via_ir — documented in the README requirements and limitations. (The
     alternative — restructuring the index API to shave stack slots — was rejected as fragile.)

2. All harness state grouped into one ReactiveEnv storage struct (spec §8 listed direct
   base-contract vars). This lets the engine libraries take the whole environment by a single storage
   reference. capturedLogs/pendingLogs must be storage (they outlive the nested external calls
   and the _react return), so a struct is required, not just convenient.

3. Per-cycle scratch stored on-chain (pendingCallbacks/deferredSubs/results in ReactiveEnv,
   cleared each run). Memory arrays can't grow across the nested trigger/deliverCallback external
   calls within one cycle, so growable storage arrays are used instead.

4. CallbackResult.success / returnData derivation. The spec defines the struct but not how to
   populate success. The real CallbackProxy swallows a failed recipient call into a
   CallbackFailure event rather than reverting, so per callback: success = deliverCallback did not revert AND no CallbackFailure(recipient) was emitted; returnData = proxy-level revert data.

5. Reverted-react() phantom-emission handling (the subtlest correctness point, not anticipated by
   the spec). Foundry's vm.getRecordedLogs() returns logs emitted inside a react() frame that then
   reverted — rolled back on-chain but still recorded. A react() that requests a callback and then
   reverts would otherwise deliver a phantom callback (§7.3 violation). The engine detects the
   ReactiveContractReverted marker trigger emits and discards the rolled-back prefix while
   processing the genuine post-catch suffix — so no phantom callback/subscription is produced, yet
   a Metered charge's real BlacklistContract is still captured and applied to the index.

6. Idempotent, position-sensitive subscription identity. The spec blessed positional matching;
   this extends positional identity to add/remove too. add is idempotent (re-subscribing
   identical criteria reactivates the existing row), mirroring the node's Subscribe
   (mem_filters.go) and making remove's "first match" unambiguous. This is intentionally stricter
   than the node's position-agnostic FNV FilterId.

7. Node-faithful minor fields. LogRecord.opCode = raw topic count (matches
   event_processor.go); cron LogRecords use emitter 0x…ffffff, chain = reactive, topic1 =
   block number, topicCount 2.

8. _react saves and restores the active chain. It runs triggers on the reactive chain and
   deliveries on each destination chain, then restores block.chainid to whatever the test had set,
   so callers aren't left on a stray destination chain.

9. Blacklist/Whitelist applied immediately (not deferred like Subscribe/Unsubscribe). This matches
   the spec's §7 switch but diverges from the node's one-block delay; documented as a deliberate,
   narrow (Metered-only) simplification.

10. recordLogs armed once. Empirically, getRecordedLogs() drains but keeps recording active, so
    the engine arms recording once in _reactiveSetUp and drains at each capture point (the spec's
    explicit re-arm calls are redundant, kept out for clarity).

11. _fund(addr, amount) = vm.deal. The spec lists the helper but not its semantics; it funds a
    contract's balance so it can pay its own Metered charges via pay(uint256).

12. Formatting / lint config. bracket_spacing = true (to match the omni dependencies' house
    style) and exclude_lints = ["screaming-snake-case-immutable"] (example mocks use idiomatic
    lowercase immutables, as real reactive contracts do).

Review

Developed under a two-reviewer gate (architecture + code-quality) verifying each phase against the
authoritative omni contracts and the node source (mem_filters.go, event_processor.go,
block_producer.go). The gate surfaced three real correctness bugs — the reverted-react() phantom
callback (decision 5), cross-round double-capture in _reactUntilQuiescent, and a follow-on dropped
BlacklistContract under Metered — each now fixed and permanently guarded by a regression test.

Not included in this PR

specs/001-omni-migration.md, Stab.sol, and REACTIVE_LEGACY_VS_OMNI.md (design/planning docs) are
intentionally excluded from the commit, as requested.

🤖 Generated with Claude Code

[daniil-parsiq]

Reviewed against specs/001-omni-migration.md and the vendored Omni contracts. The architecture is strong, and all 87 tests, formatting, build-size checks, and lint pass locally.

Three P2 findings should be addressed before merge:

1. Lifecycle guards before setup — src/ReactiveTest.sol:109

   Before _reactiveSetUp, reactiveChainId defaults to zero, so _registerContract(0, contract_) passes the chain check and marks the contract reactive even though no environment exists. Require _env.initialized and apply the same lifecycle guard consistently to _switchToChain, _react, _reactUntilQuiescent, and _emitCron. Add pre-setup regression tests.

2. Restore fee-related cheatcode state — src/ReactEngine.sol:205 and :327

   Subsidized mode sets the global Foundry transaction gas price and block base fee to zero, but run never restores them. This leaks harness state into later assertions and can make a subsequent Metered cycle accidentally charge nothing. Save tx.gasprice and block.basefee at entry and restore both before returning, with a Subsidized-to-Metered regression test.

3. Pin dependency revisions in consumer instructions — README.md:28-32

   The forge install commands use default or floating revisions; the shell comment does not select lasna2-dev. Since the harness relies on exact contract layouts and behavior, put the supported tag or commit directly in each install command, matching the gitlinks used by this PR.

[pbl64k]

This is linear in the number of active subscriptions. Why doesn't this lib use the reference algo from the pre-omni system contract? That implementation is linear in the number of criteria supported, which is effectively constant. Adding subscription dedup on lookups seems fairly simple to me.

[daniil-parsiq]

This is deliberate, and it's a fidelity choice rather than an oversight.

The library ports the Omni node's subscription matcher — MemFilterService.FindSubscribers in reactive-tornado/abci/rnk/mem_filters.go — which is itself an O(N) linear scan over TopicFilters (confirmed in the node source; the REACTIVE_LEGACY_VS_OMNI reference also notes FindSubscribers is "an O(N) linear scan" in both generations). That linear scan is the reference algorithm for Reactive Omni. The recursive trie you're pointing at lives in AbstractSubscriptionService — the legacy 1.0 on-chain subscription service, a different generation. The migration's explicit goal (spec §6.4) was to mirror what the Omni node actually does, so matching its loop (and its per-event dedup) keeps the harness byte-faithful to on-chain behavior.

On dedup: it's already done on lookups — findSubscribers returns each subscriber contract at most once via the seen-scan, which is exactly the node's processedFilterContracts guard.

On cost: this is a test library, so N is the number of subscriptions in one test — tiny. O(N) vs O(criteria) has no practical impact here, and the linear scan is simpler and more auditable than a recursive trie with nested mapping storage (which would also reintroduce the node's position-agnostic FNV FilterId collision that §6.4 deliberately avoids by matching positionally).

If there were a concrete scenario with very large synthetic subscription sets where scan cost mattered, I'd happily revisit — but the trie would trade node-fidelity and simplicity for efficiency the tests don't need.

[pbl64k]

I would argue that we need to fix the implementation in the node then. I do not know why it's implemented as a linear scan, but that means we have a serious scalability issue. The on-chain implementation predates both off-chain implementations. We certainly have massive, dynamic subscription sets as something that we have to be able to do for real world cases. Offline tests might also plausibly try stress testing reactive code in the presence of large numbers of active subscriptions.

[pbl64k]

Thank you, this looks far more practical for stress testing and/or extreme corner cases in dynamic subscriptions, while remaining completely adequate for run-of-the-mill test, and it's frankly more aesthetically pleasing than the brute force linear scan. I have no further comments before the merge.

[pbl64k]

Can we improve branch coverage for this contract?

$ forge coverage --ir-minimum -vvv --no-match-coverage ^test/
[...]

Ran 11 test suites in 1.52s (337.55ms CPU time): 87 tests passed, 0 failed, 0 skipped (87 total tests)

╭---------------------------------+------------------+------------------+----------------+-----------------╮
| File                            | % Lines          | % Statements     | % Branches     | % Funcs         |
+==========================================================================================================+
| src/ReactEngine.sol             | 95.15% (157/165) | 96.52% (194/201) | 85.00% (34/40) | 100.00% (20/20) |
|---------------------------------+------------------+------------------+----------------+-----------------|
| src/ReactiveTest.sol            | 85.11% (80/94)   | 79.82% (87/109)  | 24.24% (8/33)  | 89.47% (17/19)  |
|---------------------------------+------------------+------------------+----------------+-----------------|
| src/SubscriptionIndex.sol       | 94.44% (51/54)   | 94.44% (85/90)   | 86.67% (13/15) | 100.00% (7/7)   |
|---------------------------------+------------------+------------------+----------------+-----------------|
| src/env/EnvironmentDeployer.sol | 100.00% (11/11)  | 100.00% (13/13)  | 100.00% (0/0)  | 100.00% (2/2)   |
|---------------------------------+------------------+------------------+----------------+-----------------|
| Total                           | 92.28% (299/324) | 91.77% (379/413) | 62.50% (55/88) | 95.83% (46/48)  |
╰---------------------------------+------------------+------------------+----------------+-----------------╯

[daniil-parsiq]

Improved. Added targeted tests that raise src/ReactiveTest.sol line coverage from 85% to 94% and function coverage to 100% (--ir-minimum --no-match-coverage ^test/):

• test/CaptureHelpers.t.sol — both branches of every §6.8 capture helper (_recordedLogs, _assertEmitted ×2, _assertNotEmitted, _countEmitted, _findEmitted), including the found/absent and revert paths.
• Pre-setup lifecycle-guard reverts for _registerContract/_switchToChain/_react/_reactUntilQuiescent/_emitCron in EnvironmentGuards.t.sol (these also close @daniil-parsiq's finding Migrate reactive test library to Reactive Omni #1).

The branch % stays modest largely due to Foundry's --ir-minimum branch counting: it splits each compound &&/|| in the assertion helpers into many sub-branches, and it attributes the inlined private _requireSetUp() guard to the callee body rather than the call site (the only 6 lines still showing 0 hits are those call sites, which every test executes — the guard is exercised both ways: pass in normal tests, revert in the new pre-setup tests). I kept the shared guard over inlining the require six times, since the logic is fully tested and the DRY guard reads better; happy to inline it if you'd prefer the call-site lines to register.

[pbl64k]

inlined private _requireSetUp()

But the way, it would be more idiomatic to wrap it in a modifier, but that's a minor nitpick.

[daniil-parsiq]

Thanks for the review. All three P2 findings are addressed in 36e0604 (pushed), plus the two inline threads from @pbl64k. 102 tests pass; fmt --check / build --sizes / test green under the ci profile.

1. Lifecycle guards before setup — fixed. Added a _requireSetUp() guard (reverting "ReactiveTest: call _reactiveSetUp first") to _registerContract, _switchToChain, _react, _reactUntilQuiescent, and _emitCron (it already existed on _registerChain). So a pre-setup _registerContract(0, c) — which previously slipped through because reactiveChainId == 0 — now reverts. Added six pre-setup regression tests in EnvironmentGuards.t.sol.

2. Restore fee-related cheatcode state — fixed. ReactEngine.run now saves tx.gasprice/block.basefee on entry and restores them before returning, so Subsidized mode no longer leaks a zeroed price into later assertions or a following Metered cycle. Added test_subsidizedRestoresGasPriceSoLaterMeteredStillCharges (Subsidized → Metered with no re-set of the price → Metered still charges).

One subtlety worth flagging: the saved values must live in storage, not a local. Under via_ir the optimizer treats GASPRICE/BASEFEE as tx-invariant and re-materializes a captured local at the restore site (reading the already-zeroed value) — it doesn't model vm.txGasPrice/vm.fee as mutating them. A storage load can't be folded back into the opcode, so the restore sticks. (Verified empirically before landing.)

3. Pin dependency revisions — fixed. The README install commands now pin the exact revisions matching this repo's submodule gitlinks: [email protected] and system-smart-contracts@8f7b902700dc1a545d11b87bfb575ac06485224a.

The branch-coverage ask (@pbl64k) is addressed in its own thread — new capture-helper + guard tests raise ReactiveTest.sol line coverage 85% → 94% and functions to 100%.

[daniil-parsiq]

findSubscribers: ported to the node's wildcard-mask bucket index (947f0b3)

Follow-up to the earlier thread on the O(N) linear scan (@pbl64k). The node has since replaced its own linear scan with a 64-bucket wildcard-mask index (upstream commit "Index FindSubscribers by wildcard mask instead of linear scan"), so this library now tracks that.

What changed

• Storage model → the node's uid-keyed structure. Subscription[] is replaced by distinct-criteria filters (TopicFilter), each carrying one config per subscriber (FilterConfig), keyed by a Keccak256 FilterId. Subscribers that share criteria collapse into one filter (mirrors the node's TopicFilter.Configs).
• Matching is now O(64), not O(N). findSubscribers enumerates the 64 wildcard masks and probes each with a single O(1) map lookup, keyed by a uid built from the event's own concrete fields — at most one filter can hit per mask. Cost is bounded at 64 probes + the true matched count, independent of subscription count. A filterMatchesEvent-style field re-check is kept as a defense-in-depth backstop on each hit, exactly as the node does.
• Consensus-visible ordering change adopted. Results now come back in the node's canonical (uid, contract) order (lowest-uid filter wins attribution of a contract reachable via several matching filters), not subscription-creation order. This is the node's one deliberate consensus-visible behavior change, and the engine triggers subscribers / returns CallbackResult[] in this order, so tests observe it.

Fidelity anchor

FilterId is computed byte-for-byte as the node's computeFilterId: mask byte, then only the concrete fields, with chainId little-endian (matching U64tob), truncated to 16 bytes. I verified byte-identical uids against the node's own BuildFilterId for both a concrete-fields and an all-wildcard filter — pinned as test_uidMatchesNodeComputeFilterId. This is what makes our trigger order match what the real network would produce for the same filter set (not merely internally-consistent).

Notes

• setActive (blacklist/whitelist) uses a scan over filters rather than porting the node's byContract reverse index — it's a rare cold path and at test-scale counts the reverse index (plus its pointer-aliasing bookkeeping in Solidity) would be pure liability. Called out in the code comment.
• Filter detach on last-unsubscribe uses swap-pop; safe because findSubscribers imposes canonical order regardless of slice position (same rationale the node gives).

Tests: unit suite 26 → 30 (added ordering-independence, node uid-parity, swap-remove, fully-concrete-bucket); 106 total, all passing, clean build + forge fmt.

[daniil-parsiq]

Benchmark: linear scan vs. bucket index — findSubscribers gas by subscription count

Measured the retired O(N) linear scan against the new 64-bucket wildcard-mask index at the same scales as the node's perf_findsubscribers_test.go (100 / 10k / 1M active subscriptions). Harness added in b1258ef (test/bench/, gated behind RUN_BENCH so it's inert in CI). Metric is gas for a single findSubscribers lookup — the deterministic EVM cost, a direct proxy for compute/runtime.

Subscriptions	OLD — linear scan	NEW — bucket index	NEW vs OLD
100	92,056	155,945	1.7× more (below crossover)
10,000	9,019,690	155,945	58× cheaper
1,000,000	2,835,183,518	155,945	~18,000× cheaper

(forge 1.5.1, solc 0.8.29, via_ir; one populated mask bucket, event resolves to exactly one match.)

Reading it

• NEW is flat — 155,945 gas at every scale. The lookup is 64 bucket probes + the matched count, independent of N. 100 → 1M subscriptions: identical gas. This is the whole point of the port.
• OLD grows ~linearly, then worse. ~880 gas/row for the scan, plus a quadratic tail: the old algorithm allocates an address[](N) temp buffer, whose memory-expansion cost is N²/512. At 1M that buffer alone is ~1.95B gas (≈69% of the 2.84B total); the scan itself is the other ~880M.
• Crossover ≈ 170 subscriptions. Below that the linear scan is cheaper (fewer than ~64 effective rows beats 64 unconditional bucket probes + keccak-per-probe) — the honest trade-off I flagged when we discussed the port. Above it the gap widens without bound.
• The new index's bound is 64 probes regardless of how filters distribute across buckets (worst case = all 64 buckets occupied = 64 probes), so N-independence isn't specific to this one-bucket layout.

Methodology note (why the numbers are trustworthy). Populating N subscriptions in the measuring frame permanently grows EVM memory (each add allocates scratch), which would charge the subsequent lookup a memory-expansion penalty proportional to N — a pure artifact. The benchmark populates inside a separate BenchTarget frame (its memory bloat is discarded on return) and measures the lookup as a fresh external call. Before isolating this, the "new" number falsely appeared to grow 160k → 562k at 1M; isolated, it's a constant 155,945.

Reproduce:

RUN_BENCH=true FOUNDRY_PROFILE=bench forge test --match-path 'test/bench/*' -vv

(the 1M cases run in ~9s total.)

[pbl64k]

Thank you, this looks far more practical for stress testing and/or extreme corner cases in dynamic subscriptions, while remaining completely adequate for run-of-the-mill test, and it's frankly more aesthetically pleasing than the brute force linear scan. I have no further comments before the merge.