feat(indexer): event handler registry, stream/distribution handlers, and GraphQL schema - #49
Conversation
📝 WalkthroughWalkthroughAdds shared handler types and a registry, implements stream and distribution event handlers with payload parsers and tests, and expands the streams GraphQL schema with full types and paginated queries. ChangesCommon Handler Registry
Stream Event Handlers
Distribution Event Handlers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@pre-cious-Igwealor Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
40f3b31 to
8a08b0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
indexer/common/src/handlers/registry.test.ts (1)
90-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for handler exceptions in
dispatch.The dispatch tests currently only cover resolved handler results. Once
dispatchis hardened to catch handler rejections, add a test verifying that a throwing handler returns aHandlerResultwithok: falseandretriable: truewithout breaking the dispatch of sibling handlers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@indexer/common/src/handlers/registry.test.ts` around lines 90 - 113, The dispatch tests only cover successful handler results, so add coverage in registry.test.ts for the rejection path in HandlerRegistry.dispatch. Create a case with multiple matched handlers where one handler throws/rejects and another succeeds, then assert dispatch still invokes all handlers, returns a HandlerResult with ok: false and retriable: true for the failing handler, and preserves the sibling handler’s result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@indexer/common/src/handlers/registry.ts`:
- Around line 38-41: The `Registry.dispatch` method is currently using
`Promise.all`, so one throwing handler aborts the whole batch and hides sibling
results. Update `dispatch(event: SorobanEventInput)` to invoke each matched
handler defensively, catching exceptions around each `h(event)` call (or
attaching `.catch`) and converting failures into `{ ok: false, error, retriable:
true }`. Keep the full `HandlerResult[]` returned even when one or more handlers
fail, and preserve the existing `matches`/`EventHandler` flow.
In `@indexer/distributions/src/handlers/distribution-created.handler.ts`:
- Around line 14-28: The distribution_created handler currently returns success
after checking only distributionId, which lets malformed events pass because
parseDistributionCreated supplies defaults for other fields. In
distribution-created.handler.ts, update the validation in the
distributionCreated handler to reject missing or empty creator, token,
transactionHash, and non-positive totalAmount/recipientCount before the
console.info and success return. Use the existing payload fields from
parseDistributionCreated and return the same error shape with retriable false
when any required value is invalid.
In `@indexer/distributions/src/handlers/distribution-pause.handler.ts`:
- Around line 14-27: The pause/resume event handlers only validate
distributionId, so malformed events with empty actor or transaction hash still
succeed. Update the distribution-pause.handler and the matching resume handler
to reject missing pausedBy/resumedBy and transactionHash in addition to
distributionId, returning a non-ok response with an appropriate error message
when any required field is absent. Use the existing handler logic and symbol
names like the event payload checks and console.info logging to keep the
validation consistent across both handlers.
In `@indexer/distributions/src/handlers/tokens-claimed.handler.ts`:
- Around line 14-28: The tokens_claimed handler currently only validates
distributionId, so invalid claim events with empty claimant, transactionHash, or
defaulted amount can still pass as ok: true. Update the validation in
tokens-claimed.handler.ts to require all claim fields before logging or
returning success, using the existing handler logic around the payload checks
and the tokens-claimed event path. If any required field is missing or empty,
return a non-ok, non-retriable error like the existing missing distributionId
case, and keep the success path only for fully populated claim events.
In `@indexer/distributions/src/handlers/types.ts`:
- Around line 41-49: Guard event.data before casting it in the parser helpers
(the shared parsing logic in types.ts, including the distribution parser and the
other affected parsers at the referenced sections). Add an explicit object/null
check before using data as Record<string, unknown>, and fail fast for null or
primitive payloads with a non-retriable error path instead of letting the
handler retry forever. Keep the existing field extraction logic (str/num and the
distributionId/transactionHash aliases) unchanged once the payload is validated.
In `@indexer/streams/src/handlers/types.ts`:
- Around line 24-55: The parse helpers in parseStreamFunded,
parseStreamWithdrawal, and parseStreamCancel are fabricating default values for
required payload fields, which lets incomplete events pass as valid. Update
these functions to preserve missing properties as undefined or throw/return a
parse failure instead of filling in "" or "0", and ensure the handler paths that
consume these payloads reject malformed events rather than acknowledging them as
successful.
- Around line 24-55: parseStreamFunded, parseStreamWithdrawal, and
parseStreamCancel currently cast unknown data to Record<string, unknown> without
guarding against null, so property access can throw and get misclassified as
retriable. Add an explicit object/non-null check before dereferencing in these
parser helpers, and if the payload is invalid, send it through the same
non-retriable validation path used for other malformed inputs so dispatch does
not keep retrying permanently bad events.
---
Nitpick comments:
In `@indexer/common/src/handlers/registry.test.ts`:
- Around line 90-113: The dispatch tests only cover successful handler results,
so add coverage in registry.test.ts for the rejection path in
HandlerRegistry.dispatch. Create a case with multiple matched handlers where one
handler throws/rejects and another succeeds, then assert dispatch still invokes
all handlers, returns a HandlerResult with ok: false and retriable: true for the
failing handler, and preserves the sibling handler’s result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a74faf0-654b-40c6-89b5-db590cbec09e
📒 Files selected for processing (20)
indexer/common/src/handlers/index.tsindexer/common/src/handlers/registry.test.tsindexer/common/src/handlers/registry.tsindexer/common/src/handlers/types.tsindexer/common/src/index.tsindexer/distributions/src/handlers/distribution-created.handler.tsindexer/distributions/src/handlers/distribution-handlers.test.tsindexer/distributions/src/handlers/distribution-pause.handler.tsindexer/distributions/src/handlers/index.tsindexer/distributions/src/handlers/tokens-claimed.handler.tsindexer/distributions/src/handlers/types.tsindexer/distributions/src/index.tsindexer/streams/schema.graphqlindexer/streams/src/handlers/index.tsindexer/streams/src/handlers/stream-cancel.handler.tsindexer/streams/src/handlers/stream-funded.handler.tsindexer/streams/src/handlers/stream-handlers.test.tsindexer/streams/src/handlers/stream-withdrawal.handler.tsindexer/streams/src/handlers/types.tsindexer/streams/src/index.ts
|
Thank you for your awesome contribution, however after analyzing your implementation, there are some minor fixes and merge conflict. Kindly fix them to merge your PR asap. Also do not forget to use fundable.finance to offramp. |
…ble-Protocol#30) - Define `SorobanEventInput`, `HandlerResult`, `EventHandler`, and `HandlerFilter` types in `indexer/common/src/handlers/types.ts` - Implement `HandlerRegistry` in `registry.ts` with `register()`, `matches()` (filters by contractId, topic, eventName), and `dispatch()` - Export all handler types and the registry from `common/src/index.ts` - Add 10 unit tests covering filter matching, dispatch, error results, fluent chaining, and the empty-filter catch-all
…rotocol#33) Expand the stub schema to cover the full public API shape: - `Stream` type with all fields (status, balances, timestamps, relations) - `WithdrawalAction` and `CancelAction` types - `StreamStatus` enum (ACTIVE / CANCELLED / COMPLETED) - `StreamFilterInput` and `PaginationInput` input types - `StreamConnection` / `PageInfo` for cursor-based pagination - `Query` type with `stream`, `streams`, `streamsByRecipient`, `streamsBySender` root fields
…ers (Fundable-Protocol#35) - Add payload types and parser helpers (`parseStreamFunded`, `parseStreamWithdrawal`, `parseStreamCancel`) in `handlers/types.ts` - Implement `streamFundedHandler`, `streamWithdrawalHandler`, and `streamCancelHandler` — each validates required fields, logs an info line, and returns `{ ok: true }` (or a non-retriable error for missing streamId); DB persistence is left as TODO pending Fundable-Protocol#32 - Export all handlers from `streams/src/index.ts` - Add 7 unit tests with mocked payloads covering valid events and missing-streamId error paths for all three handlers
…undable-Protocol#38) - Add payload types and parsers for distribution-created, tokens-claimed, distribution-paused, and distribution-resumed events in `handlers/types.ts` - Implement `distributionCreatedHandler`, `tokensClaimedHandler`, `distributionPausedHandler`, and `distributionResumedHandler` — each validates the required distributionId, logs, and returns `{ ok: true }`; DB persistence is TODO pending Fundable-Protocol#36 (repository layer) and Fundable-Protocol#27 (event store) - Export all handlers from `distributions/src/index.ts` - Add 9 unit tests covering valid payloads, missing-distributionId errors, and an idempotency test for `distributionCreatedHandler`
- registry: catch per-handler rejections in dispatch() so one failure does not abort the entire batch (Promise.all propagation fix) - distributions/types: add record() guard before dereferencing event.data to prevent poison-message loops on null/primitive payloads - distribution-created: validate creator, token, totalAmount, transactionHash, and recipientCount > 0 in addition to distributionId - distribution-pause: validate pausedBy/resumedBy and transactionHash for both pause and resume handlers - tokens-claimed: validate claimant, transactionHash, and non-zero amount - streams/types: replace fabricated "" / "0" defaults with undefined for absent fields using a safe record() coercion helper and nullable str() - stream handlers: add missing field validation for sender/recipient/ cancelledBy, amount, token, and transactionHash
8a08b0e to
5621be3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@indexer/streams/src/handlers/types.ts`:
- Around line 28-31: The str helper in types.ts is coercing any unknown value
with String(v), which lets objects/arrays become truthy strings and slip through
parseStreamFunded and parseStreamWithdrawal as valid required fields. Update str
to reject non-scalar inputs by returning undefined for objects and arrays, or
add explicit type checks before coercion, so only real scalar values are
accepted when building the parsed payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9954e4a4-49dd-4386-bc5f-267ad1f452c1
📒 Files selected for processing (20)
indexer/common/src/handlers/index.tsindexer/common/src/handlers/registry.test.tsindexer/common/src/handlers/registry.tsindexer/common/src/handlers/types.tsindexer/common/src/index.tsindexer/distributions/src/handlers/distribution-created.handler.tsindexer/distributions/src/handlers/distribution-handlers.test.tsindexer/distributions/src/handlers/distribution-pause.handler.tsindexer/distributions/src/handlers/index.tsindexer/distributions/src/handlers/tokens-claimed.handler.tsindexer/distributions/src/handlers/types.tsindexer/distributions/src/index.tsindexer/streams/schema.graphqlindexer/streams/src/handlers/index.tsindexer/streams/src/handlers/stream-cancel.handler.tsindexer/streams/src/handlers/stream-funded.handler.tsindexer/streams/src/handlers/stream-handlers.test.tsindexer/streams/src/handlers/stream-withdrawal.handler.tsindexer/streams/src/handlers/types.tsindexer/streams/src/index.ts
✅ Files skipped from review due to trivial changes (3)
- indexer/common/src/handlers/index.ts
- indexer/distributions/src/handlers/index.ts
- indexer/streams/src/handlers/index.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- indexer/streams/src/index.ts
- indexer/common/src/index.ts
- indexer/common/src/handlers/registry.test.ts
- indexer/distributions/src/handlers/types.ts
- indexer/distributions/src/index.ts
- indexer/streams/src/handlers/stream-cancel.handler.ts
- indexer/streams/src/handlers/stream-handlers.test.ts
- indexer/common/src/handlers/types.ts
- indexer/streams/schema.graphql
- indexer/distributions/src/handlers/distribution-handlers.test.ts
- indexer/common/src/handlers/registry.ts
| function str(v: unknown): string | undefined { | ||
| if (v === undefined || v === null || v === "") return undefined; | ||
| const s = String(v); | ||
| return s === "" ? undefined : s; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject non-scalar payload fields instead of stringifying them.
String(v) turns malformed values like {} into "[object Object]", so parseStreamFunded / parseStreamWithdrawal can produce truthy required fields from invalid payloads and the handlers will acknowledge them as { ok: true }. Return undefined for objects/arrays here, or validate field types explicitly before coercion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@indexer/streams/src/handlers/types.ts` around lines 28 - 31, The str helper
in types.ts is coercing any unknown value with String(v), which lets
objects/arrays become truthy strings and slip through parseStreamFunded and
parseStreamWithdrawal as valid required fields. Update str to reject non-scalar
inputs by returning undefined for objects and arrays, or add explicit type
checks before coercion, so only real scalar values are accepted when building
the parsed payload.
Summary
This PR establishes the full event-processing layer for the Fundable Protocol indexer: a reusable
HandlerRegistryin the common package, a complete Streams GraphQL schema, typed stream event handlers (funded, withdrawal, cancel), and typed distribution event handlers (created, tokens-claimed, paused, resumed). All handlers are functional stubs that validate required fields and log events, with TODO markers pointing to the open dependency issues (#27, #32, #36) that will add DB persistence in follow-up PRs.closes #30
closes #33
closes #35
closes #38
Changes
Add event handler registration interface #30 — Handler registration interface (
indexer/common): AddsSorobanEventInput,HandlerResult,EventHandler, andHandlerFiltertypes incommon/src/handlers/types.ts. ImplementsHandlerRegistryinregistry.tswithregister(),matches()(filter by contractId / topic / eventName), anddispatch()methods and a fluent API. Exported fromcommon/src/index.ts. Includes 10 unit tests covering filter matching, dispatch routing, chaining, error propagation, and catch-all (empty-filter) handlers.Define Streams GraphQL schema #33 — Streams GraphQL schema (
indexer/streams): Replaces the 12-line stub with a full schema:StreamStatusenum,Stream,WithdrawalAction,CancelActiontypes with all expected fields;StreamFilterInputandPaginationInputinputs;StreamConnection/PageInfopagination types; andQueryroot withstream,streams,streamsByRecipient,streamsBySenderresolvers.Implement stream funding, withdrawal, and cancel handlers #35 — Stream event handlers (
indexer/streams): Adds payload types and parser helpers (parseStreamFunded,parseStreamWithdrawal,parseStreamCancel) inhandlers/types.ts. ImplementsstreamFundedHandler,streamWithdrawalHandler, andstreamCancelHandler— each validatesstreamId, logs the event, and returns{ ok: true }. MissingstreamIdreturns{ ok: false, retriable: false }. DB persistence is TODO pending Define Streams database schema #32. 7 unit tests cover valid events and missing-streamId paths for all three handlers.Implement distribution event handlers #38 — Distribution event handlers (
indexer/distributions): Adds payload types and parsers for the four distribution events. ImplementsdistributionCreatedHandler,tokensClaimedHandler,distributionPausedHandler, anddistributionResumedHandler— same validation/logging/error pattern as stream handlers. DB persistence is TODO pending Define Distributions database schema #36 and Implement indexed event repository #27. 9 unit tests cover valid payloads, missing-distributionId errors, and an idempotency test.Test plan
vitest) live inregistry.test.ts,stream-handlers.test.ts, anddistribution-handlers.test.ts.Not built/tested locally due to bun not being installed in the CI runner environment. All handler implementations use
console.infoand return typed results only — no side effects until the DB repository PRs (#27, #32, #36) land and are wired in.Summary by CodeRabbit
New Features
Bug Fixes