test: ethflow error-path coverage + manifest topic-0 assertions by jean-neiverth · Pull Request #164 · nullislabs/shepherd

jean-neiverth · 2026-07-03T19:12:44Z

Summary

Add 429 (rate-limited) and malformed-JSON-body (200 with garbage) test cases to ethflow-watcher strategy
Add a manifest_topic0_matches_* test to both ethflow-watcher and twap-monitor that asserts the hardcoded event_signature in module.toml equals the SIGNATURE_HASH constant from the cowprotocol crate

Test plan

14 ethflow-watcher tests pass (2 new + 12 existing)
twap-monitor topic-0 assertion passes
cargo fmt and cargo clippy clean

Closes #60
Closes #61

Add 429 rate-limiting and malformed-JSON-body test cases to ethflow-watcher's observe path. Both verify the strategy handles degraded orderbook responses without writing false markers. Add a compile-time assertion per module that the hardcoded event_signature in module.toml matches the keccak256 SIGNATURE_HASH from the cowprotocol crate, so an ABI change or typo breaks the build instead of silently missing on-chain events. Closes #60 Closes #61

lgahdl

Solid additions — the topic-0 pin tests are especially valuable (a silently mismatched event_signature would drop all on-chain events). Two small comments and one doc nit.

lgahdl · 2026-07-08T14:57:08Z

+    fn manifest_topic0_matches_order_placement_signature_hash() {
+        let manifest = include_str!("../module.toml");
+        let expected = format!("0x{:x}", OrderPlacement::SIGNATURE_HASH);
+        assert!(


manifest.contains(&expected) searches the whole file as a raw string — today it only matches because the hash appears once, but a future TOML addition (comment, second subscription, etc.) could produce a false positive. Anchoring to the field name makes the intent explicit:

Suggested change

assert!(

assert!(

manifest.contains(&format!("event_signature = \"{expected}\"")),

lgahdl · 2026-07-08T14:57:08Z

+
+    /// 200 with a malformed (non-JSON) body → the observe path still
+    /// marks the order as observed, because the strategy only checks
+    /// the HTTP status, not the body structure.


The comment describes what the strategy does, not why. The reason the body is irrelevant is that HTTP 200 is the orderbook's contract guarantee that the order is indexed — the response body is an implementation detail, not part of that contract.

Suggested change

/// the HTTP status, not the body structure.

/// marks the order as observed. HTTP 200 is the orderbook's contract

/// guarantee that the order is indexed; the response body is not

/// part of that contract.

lgahdl · 2026-07-08T14:57:08Z

+        let expected = format!("0x{:x}", ConditionalOrderCreated::SIGNATURE_HASH);
+        assert!(
+            manifest.contains(&expected),
+            "module.toml event_signature must equal ConditionalOrderCreated::SIGNATURE_HASH\n\


Same manifest.contains raw-string concern as in ethflow-watcher — anchor to the TOML field to make the assertion precise:

Suggested change

"module.toml event_signature must equal ConditionalOrderCreated::SIGNATURE_HASH\n\

assert!(

manifest.contains(&format!("event_signature = \"{expected}\"")),

lgahdl

Good coverage additions. One assertion is fragile; one minor note on the topic-0 guard.

lgahdl · 2026-07-08T15:05:06Z

+            .filter(|l| l.message.contains("indexer check failed"))
+            .collect();
+        assert_eq!(warn_lines.len(), 1);
+        assert_eq!(warn_lines[0].level, LogLevel::Warn);


SdkHostError's Display may render the message field ("Too Many Requests") without the numeric code, in which case this assertion fails even when the behaviour is correct. Use "Too Many Requests" instead, or drop the string check and rely on the level + empty-store assertions which already capture the important invariant.

Suggested change

assert_eq!(warn_lines[0].level, LogLevel::Warn);

assert!(warn_lines[0].message.contains("Too Many Requests"));

lgahdl · 2026-07-08T15:05:07Z

+    fn manifest_topic0_matches_order_placement_signature_hash() {
+        let manifest = include_str!("../module.toml");
+        let expected = format!("0x{:x}", OrderPlacement::SIGNATURE_HASH);
+        assert!(


Minor: manifest.contains(&expected) passes if the hash string appears anywhere in the manifest (comment, another field). In practice a 32-byte hex won't show up elsewhere, but parsing the TOML and comparing event_signature directly would make the test self-documenting and immune to that edge case.

Reads the shipped module.toml via include_str! and asserts its pinned event_signature equals the OrderPlacement / ConditionalOrderCreated SIGNATURE_HASH constant - a manifest/code drift the existing ABI-hash guard cannot catch. Ported from #164 before closing it as superseded.

mfw78 · 2026-07-15T07:19:22Z

Superseded by #343. Both add the same #60/#61 coverage (topic-0 guards + EthFlow 429 / malformed-body tests), but this PR is pinned to the pre-refactor on_logs / SdkHostError / 3-arg decode_order_placement API that no longer exists on develop, so it would need a full rewrite to compile. #343 carries the equivalent tests against the current API. Your stronger idea — asserting the shipped module.toml event_signature via include_str! (catches a manifest/code drift the ABI-hash constant check can't) — has been ported into #343 for both modules (commit f3bdbff). Closing in favour of #343.

Reads the shipped module.toml via include_str! and asserts its pinned event_signature equals the OrderPlacement / ConditionalOrderCreated SIGNATURE_HASH constant - a manifest/code drift the existing ABI-hash guard cannot catch. Ported from #164 before closing it as superseded.

…ts (#343) * test(#60,#61): assert topic-0 values and add EthFlow error-path tests Fixes #60: both twap-monitor and ethflow-watcher now assert at test time that the hardcoded topic-0 in module.toml equals keccak256 of the canonical event ABI string via alloy_sol_types::SolEvent::SIGNATURE_HASH. Fixes #61: adds two missing coverage paths in ethflow-watcher — - 429 rate-limit: verify Warn is emitted and no observed: marker is written (distinguishes from the 404 indexer-lag path) - malformed JSON body with HTTP 200: verify the strategy writes the observed: marker because it treats Ok(_) as confirmation without parsing the response body * test: assert module.toml event_signature matches the code SIGNATURE_HASH Reads the shipped module.toml via include_str! and asserts its pinned event_signature equals the OrderPlacement / ConditionalOrderCreated SIGNATURE_HASH constant - a manifest/code drift the existing ABI-hash guard cannot catch. Ported from #164 before closing it as superseded. --------- Co-authored-by: Luiz Gustavo Abou Hatem de Liz <luizgustavoahsc@gmail.com>

jean-neiverth marked this pull request as ready for review July 3, 2026 20:48

jean-neiverth requested a review from mfw78 as a code owner July 3, 2026 20:48

lgahdl reviewed Jul 8, 2026

View reviewed changes

mfw78 mentioned this pull request Jul 9, 2026

runtime: complete execution and lifecycle hardening #294

Closed

lgahdl mentioned this pull request Jul 14, 2026

ci: sccache R2 config (#336) hard-fails every fork PR — secrets don't flow to fork pull_request runs #337

Open

mfw78 added the base: develop Open PR targets develop — merge FIRST, before the dev/m1 integration PRs. label Jul 15, 2026

mfw78 mentioned this pull request Jul 15, 2026

test(#60,#61): topic-0 keccak guards + EthFlow 429/malformed-body tests #343

Merged

2 tasks

mfw78 closed this Jul 15, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test: ethflow error-path coverage + manifest topic-0 assertions#164

test: ethflow error-path coverage + manifest topic-0 assertions#164
jean-neiverth wants to merge 1 commit into
nullislabs:developfrom
bleu:fix/60-61-ethflow-tests-and-topic-assertions

jean-neiverth commented Jul 3, 2026 •

edited by mfw78

Loading

Uh oh!

lgahdl left a comment

Uh oh!

lgahdl Jul 8, 2026

Uh oh!

lgahdl Jul 8, 2026

Uh oh!

lgahdl Jul 8, 2026

Uh oh!

lgahdl left a comment

Uh oh!

lgahdl Jul 8, 2026

Uh oh!

lgahdl Jul 8, 2026

Uh oh!

mfw78 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	assert!(
	assert!(
	manifest.contains(&format!("event_signature = \"{expected}\"")),

-    /// the HTTP status, not the body structure.
+    /// marks the order as observed. HTTP 200 is the orderbook's contract
+    /// guarantee that the order is indexed; the response body is not
+    /// part of that contract.

	"module.toml event_signature must equal ConditionalOrderCreated::SIGNATURE_HASH\n\
	assert!(
	manifest.contains(&format!("event_signature = \"{expected}\"")),

	assert_eq!(warn_lines[0].level, LogLevel::Warn);
	assert!(warn_lines[0].message.contains("Too Many Requests"));

Uh oh!

Conversation

jean-neiverth commented Jul 3, 2026 • edited by mfw78 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

lgahdl left a comment

Choose a reason for hiding this comment

Uh oh!

lgahdl Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

lgahdl Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

lgahdl Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

lgahdl left a comment

Choose a reason for hiding this comment

Uh oh!

lgahdl Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

lgahdl Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

mfw78 commented Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jean-neiverth commented Jul 3, 2026 •

edited by mfw78

Loading