feat(sui-indexer): add Ika coordinator event polling for signature completion tracking#301
feat(sui-indexer): add Ika coordinator event polling for signature completion tracking#301
Conversation
…mpletion tracking Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
Reviewer's GuideExtends the Sui indexer to poll and process both NBTC and Ika coordinator events by introducing coordinator package support, module-aware GraphQL event fetching, new Ika event handlers, and schema/config changes to track coordinator package IDs. ER diagram for setups table with coordinator_pkgerDiagram
setups {
integer id PK
text sui_network
text nbtc_pkg
text coordinator_pkg
integer is_active
}
Updated class diagram for Sui indexer polling and Ika event handlingclassDiagram
class Processor {
- netCfg NetworkConfig
- storage IndexerStorage
- eventFetcher EventFetcher
+ constructor(netCfg NetworkConfig, storage IndexerStorage, eventFetcher EventFetcher)
+ pollAllEvents(nbtcPkgs PkgCfg[]) Promise~void~
}
class SuiEventHandler {
- storage IndexerStorage
- setupId number
+ constructor(storage IndexerStorage, setupId number)
+ handleEvents(events SuiEventNode[]) Promise~void~
+ handleIkaEvents(events SuiEventNode[]) Promise~void~
- handleCompletedSign(e SuiEventNode) Promise~void~
- handleRejectedSign(e SuiEventNode) Promise~void~
}
class IndexerStorage {
- db any
+ getActiveNbtcPkgs(networkName string) Promise~PkgCfg[]~
+ getMultipleSuiGqlCursors(ids number[]) Promise~Record~number,string~~
}
class EventFetcher {
<<interface>>
+ fetchEvents(packages FetchPackage[]) Promise~Record~string,EventsBatch~~
}
class SuiGraphQLClient {
- client GraphQLClient
+ fetchEvents(packages FetchPackage[]) Promise~Record~string,EventsBatch~~
}
class FetchPackage {
+ id string
+ module string
+ cursor string | null
}
class EventsBatch {
+ events SuiEventNode[]
+ endCursor string
+ hasNextPage boolean
}
class PkgCfg {
+ id number
+ nbtc_pkg string
+ coordinator_pkg string
}
class IkaCompletedSignEventRaw {
+ sign_id string
+ signature number[]
+ is_future_sign boolean
}
class IkaRejectedSignEventRaw {
+ sign_id string
+ is_future_sign boolean
}
class SuiEventNode {
+ type string
+ json any
+ txDigest string
}
class NetworkConfig {
+ name string
}
Processor --> IndexerStorage : uses
Processor --> EventFetcher : uses
SuiGraphQLClient ..|> EventFetcher
Processor --> SuiEventHandler : creates
SuiEventHandler --> IndexerStorage : uses
IndexerStorage --> PkgCfg : returns
SuiEventHandler --> IkaCompletedSignEventRaw : parses
SuiEventHandler --> IkaRejectedSignEventRaw : parses
EventFetcher --> EventsBatch : returns
EventsBatch --> SuiEventNode : contains
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Using
cursorId: -pkg.idand then persisting it assetupId: p.cursorIdchanges the meaning ofsetupIdfor cursors and may conflict with DB constraints/foreign keys; consider explicitly separating cursor namespaces (e.g., an extramodule/kindcolumn or composite key) instead of overloadingsetupIdwith negative values. - The event type checks in
handleIkaEventsrely one.type.includes("::coordinator_inner::CompletedSignEvent")/RejectedSignEvent; if the type format is known, it would be safer to compare against the full expected type string to avoid accidentally matching unrelated events. - The string literal
"coordinator_inner"is used in multiple places (GraphQL filters and handler type checks); consider extracting it (and the full event type suffixes) into shared constants to keep the filtering and handling logic in sync and reduce the risk of typos.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `cursorId: -pkg.id` and then persisting it as `setupId: p.cursorId` changes the meaning of `setupId` for cursors and may conflict with DB constraints/foreign keys; consider explicitly separating cursor namespaces (e.g., an extra `module`/`kind` column or composite key) instead of overloading `setupId` with negative values.
- The event type checks in `handleIkaEvents` rely on `e.type.includes("::coordinator_inner::CompletedSignEvent")`/`RejectedSignEvent`; if the type format is known, it would be safer to compare against the full expected type string to avoid accidentally matching unrelated events.
- The string literal `"coordinator_inner"` is used in multiple places (GraphQL filters and handler type checks); consider extracting it (and the full event type suffixes) into shared constants to keep the filtering and handling logic in sync and reduce the risk of typos.
## Individual Comments
### Comment 1
<location> `packages/sui-indexer/src/processor.ts:67-76` </location>
<code_context>
+ for (const p of allPackages) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid advancing the cursor when event handling fails to prevent data loss.
Because the try/catch now wraps `handleEvents` / `handleIkaEvents`, a thrown error still leads to advancing the cursor:
```ts
try {
const handler = new SuiEventHandler(this.storage, p.setupId);
if (p.isCoordinator) {
await handler.handleIkaEvents(result.events);
} else {
await handler.handleEvents(result.events);
}
} catch (error) {
logError(...);
}
if (result.endCursor && result.endCursor !== cursors[p.cursorId]) {
cursorsToSave.push({ setupId: p.cursorId, cursor: result.endCursor });
cursors[p.cursorId] = result.endCursor;
}
```
If the handler throws, those events are effectively dropped and never reprocessed. Please ensure the cursor is only advanced on successful processing (e.g., move the cursor update into the `try` after the `await`s, or otherwise skip cursor updates when a package fails).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
sczembor
left a comment
There was a problem hiding this comment.
lets improve the namings a little bit. If we are looping through all packages, lets just call it packages rather than allPackages it doesnt tell us anything extra
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
sczembor
left a comment
There was a problem hiding this comment.
lets exctract the ika events logic to a separte function, the pollEvetns method is too complex
| setupId: this.setupId, | ||
| }); | ||
|
|
||
| // TODO: Call redeem_solver service binding to record signature |
| setupId: this.setupId, | ||
| }); | ||
|
|
||
| // TODO: Call redeem_solver service binding to mark signature as rejected |
| // Get coordinator package from Ika SDK | ||
| const network = this.netCfg.name; | ||
| const coordinatorPkg = | ||
| network === "mainnet" || network === "testnet" |
There was a problem hiding this comment.
lets add a note why we only handle two networks here
|
|
||
| if (coordinatorPkg) { | ||
| packages.push({ | ||
| cursorId: -1, |
There was a problem hiding this comment.
could you explain the -1? i am a little bit lost
There was a problem hiding this comment.
I use -1 to avoid collisions with the nBTC cursor IDs
There was a problem hiding this comment.
could you give an example of how it works
| }); | ||
| } | ||
|
|
||
| if (coordinatorPkg) { |
There was a problem hiding this comment.
lets extract the ika events to a separate function and just call it from here
|
I think the description is outdated |
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
Signed-off-by: Rayane Charif <rayane.charif@gonative.cc>
|
based on a PP with Stan, this pr was false and decided to start from scratch : #322 |
Closes: #297
During deployment :
Summary by Sourcery
Generalized GraphQL event fetching to support per-module queries (not hardcoded to nbtc)
Added separate pollIkaEvents() that polls coordinator_inner::CompletedSignEvent and RejectedSignEvent
Event handlers log outcomes; actual recordIkaSig calls are deferred to a follow-up PR
Coordinator package ID resolved from @ika.xyz/sdk (mainnet/testnet only)
DB migration at deploy time:
UPDATE setups SET coordinator_pkg = '...' WHERE sui_network = 'testnet';
UPDATE setups SET coordinator_pkg = '...' WHERE sui_network = 'mainnet';