Refactor UI contract API into dedicated reporting, truth-auction, and fork modules#365
Conversation
- Extracted reporting logic (deposits, escrow ops, and fork proof helpers) from `ui/ts/contracts.ts` into `ui/ts/contracts/reporting.ts` - Moved truth-auction query helpers into `ui/ts/contracts/truthAuctions.ts` - Moved fork-auction action execution helper into `ui/ts/contracts/securityPoolActions.ts` - Added shared decoders/type helpers in `ui/ts/contracts/decoders.ts` and updated imports/exports in `ui/ts/contracts.ts` - Updated `ui/ts/tests/contracts.test.ts` to match the reorganized contract module structure
- Add regression test ensuring `loadTruthAuctionTickBidPage` throws on invalid ABI-shaped bid page data - Prevent trusting unchecked fields like malformed `refunded` in `getBidPageAtTick` responses
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
CI Agent Review
The refactoring extracts contract-interaction code from the monolithic contracts.ts into focused modules (reporting.ts, truthAuctions.ts, securityPoolActions.ts, decoders.ts) and replaces unsafe type assertions with runtime validation. One architectural concern remains: requireBigintValue is duplicated in both contracts.ts (local, still actively used) and the new decoders.ts (exported), and requireBigintArray was not extracted to decoders.ts at all, fragmenting the validation API surface across two locations.
| return BigInt(pageIndex) * BigInt(pageSize) | ||
| } | ||
|
|
||
| function requireBigintValue(value: unknown, context: string) { |
There was a problem hiding this comment.
requireBigintValue is duplicated here and in the new decoders.ts (lines 19–22). Both implementations are byte-for-byte identical, but contracts.ts does not import from decoders.ts — the local copy is still actively used by loadOracleManagerDetails (4 calls to requireBigintValue, 1 call to requireBigintArray). Additionally, requireBigintArray was not extracted to decoders.ts at all, further splitting the validation API. Consider removing both functions from contracts.ts and importing them from decoders.ts to consolidate all response-validation primitives in a single module. As-is, any future change to validation logic must be applied in two places, and new code added to contracts.ts will naturally use the local copy rather than the centralized one.
|
Reviewer note: this PR intentionally makes malformed ABI/page reads fail loudly at the UI contract boundary. The latest follow-up also reuses a shared |
|
/review |
There was a problem hiding this comment.
CI Agent Review
No issues found. The refactoring decomposes contracts.ts into focused modules and replaces unsafe type casts with runtime validation — all four agents confirmed no architectural degradations, security vulnerabilities, runtime bugs, or backdoors.
1 participant
Summary
ui/ts/contracts.tsinto focused modules:ui/ts/contracts/reporting.tsfor reporting and fork-carried escalation logic.ui/ts/contracts/truthAuctions.tsfor truth-auction read helpers.ui/ts/contracts/securityPoolActions.tsfor shared security-pool action helpers.ui/ts/contracts/decoders.tsfor small shared runtime decoders at ABI/page-read boundaries.ui/ts/contracts/forkData.tsso every current security-poolforkDataread goes through one validated tuple decoder.ui/ts/contracts.ts.Behavior Change
Unexpected ... responseerrors instead of being silently filtered, dropped, or trusted through unchecked casts.Scope
contracts.tsdomains, such as OpenOracle, trading, migration, and vault writes, are left for follow-up PRs to keep this review focused.Testing
cd ui && bun x tsc --project tsconfig.jsonbun run test-> 1360 pass, 11 skip, 0 failbun run format-> no fixes applied on final runbun run check-> cleanbun run knip-> cleangit fetch origin main && git merge origin/main-> already up to date on final check