Skip to content

Replace explicit any with proper types in onchain/soroban (1/3)#147

Open
noevidence1017 wants to merge 1 commit into
ChainForgee:mainfrom
noevidence1017:fix/explicit-any-type-safety
Open

Replace explicit any with proper types in onchain/soroban (1/3)#147
noevidence1017 wants to merge 1 commit into
ChainForgee:mainfrom
noevidence1017:fix/explicit-any-type-safety

Conversation

@noevidence1017

@noevidence1017 noevidence1017 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Re-scoped per review (#147 was 41 files). This PR is now just the onchain / Soroban group; the other two groups are staged as separate branches and will follow as their own PRs once this lands:

  • fix/type-safety-express-middleware — request/express middleware, interceptors, guards, express.d.ts
  • fix/type-safety-prisma-filters — global exception filter, Prisma-backed services, frontend

The @typescript-eslint/no-explicit-any warnerror flip has been removed from this PR and will come last as its own one-line change, once the refactors are on main (so it doesn't break other open PRs/forks).

What's in this PR (onchain only)

  • Discriminated union for OnchainJobData/OnchainJobInput so onchain.processor.ts narrows params/result per operation type.
  • unknown + a minimal SorobanErrorLike interface in soroban-error.mapper.ts instead of duck-typed any.
  • Concrete adapter return types across onchain.adapter.ts, soroban.adapter.ts, soroban-onchain.adapter.ts, ledger-backfill/reconciliation.service.ts, aid-escrow.controller.ts.

Addressing review feedback

  1. Split — done (3 groups; this is 1/3).
  2. Lint flip held back — removed from this PR.
  3. Casts at the Soroban boundary — audited. The unchecked pkg?.status as AidPackage['status'] in soroban-onchain.adapter.ts is replaced with a parsePackageStatus() guard (mirroring soroban.adapter.ts), and the AidPackage.metadata / String(unknown) boundary conversions now go through a small contract-value helper (toContractString/toStringRecord) instead of [object Object]-prone casts.

Note: the original branch did not compile or lint cleanly (the tsc/lint boxes were never checked). This version fixes the latent issues that proper typing surfaced — e.g. controller handlers now type-check their concrete return types, the processor switch is exhaustive, and ledger-backfill skips entries with no resolvable (required) campaignId.

Verification

  • tsc --noEmit clean
  • eslint clean (0 errors) on the onchain files
  • onchain Jest suites pass

Closes #99

@kilodesodiq-arch kilodesodiq-arch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @noevidence1017 — the type cleanup is in the right direction, but Id like this split before merging:

  1. The 41-file blast radius is too big for one PR. Please split into 2–3 PRs grouped by area (e.g. onchain adapters & soroban mappings, request/express middleware, error filters).
  2. Please hold off on flipping @typescript-eslint/no-explicit-any from warn to error inside this PR. That change will instantly fail CI for every other open PR (and any fork). Bring the lint rule flip in as its own one-line PR once the type refactors have landed on main.
  3. Sanity-check the new interfaces for as SomeType casts that may be masking real shape mismatches at the Soroban / Prisma boundaries.

Re-review once split.

Scopes the type-safety work to the onchain adapters, processor, services
and Soroban error mapping (split out from the original 41-file PR per
review). Does NOT flip the @typescript-eslint/no-explicit-any rule — that
will land as its own one-line PR once the type refactors are merged.

- Introduce a discriminated union for OnchainJobData/OnchainJobInput so the
  processor narrows params/result per operation type.
- Replace duck-typed any error handling in soroban-error.mapper.ts with
  unknown plus a minimal SorobanErrorLike interface.
- Validate the AidPackage status decoded from RPC against the known union
  (parsePackageStatus) instead of an unchecked `as` cast at the Soroban
  boundary.
- Add a shared contract-value helper (toContractString/toStringRecord) to
  coerce unknown RPC values without [object Object] stringification, and
  fix the AidPackage.metadata Record<string,string> mismatch.
- Skip ledger-backfill entries with no resolvable campaignId (required on
  BalanceLedger) rather than failing the whole range.
- Make AidEscrowController handlers and the onchain processor exhaustive so
  concrete return types type-check.

Verified: tsc --noEmit clean, eslint clean (0 errors), onchain Jest
suites pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@noevidence1017 noevidence1017 force-pushed the fix/explicit-any-type-safety branch from 668acb1 to 16cd758 Compare June 21, 2026 14:38
@noevidence1017 noevidence1017 changed the title Replace explicit any with proper types across onchain, auth, and error handling Replace explicit any with proper types in onchain/soroban (1/3) Jun 21, 2026
@noevidence1017

Copy link
Copy Markdown
Contributor Author

Thanks for the review @kilodesodiq-arch — addressed all three points:

  1. Split — this PR is now scoped to just the onchain / Soroban group (10 files + 1 small helper). The other two groups are ready on separate branches and I'll open them as follow-up PRs once this lands, to keep each reviewable:

    • request/express middleware + interceptors + guards + express.d.ts
    • global exception filter + Prisma services + frontend
  2. Lint rule flip — removed the no-explicit-any warnerror change from this PR. I'll send it as its own one-line PR after the three refactors are merged, so it doesn't red-CI other open PRs/forks.

  3. Casts at the Soroban/Prisma boundary — audited them. The one that was genuinely masking a shape mismatch was pkg?.status as AidPackage['status'] in soroban-onchain.adapter.ts — an unchecked cast of an RPC value to the status union. Replaced with a parsePackageStatus() guard (matching the validation already in soroban.adapter.ts). I also routed the AidPackage.metadata and String(unknown) boundary conversions through a small contract-value helper rather than casting.

One heads-up while doing this: the original branch didn't actually compile or lint (tsc/eslint were never run). Removing any surfaced real latent issues, which I fixed in this group — e.g. the controller handlers now satisfy their concrete return types, the processor switch is exhaustive, and ledger-backfill now skips entries with no resolvable campaignId (it's required on BalanceLedger) instead of trying to insert undefined. tsc --noEmit, eslint (0 errors), and the onchain Jest suites are all green now. Ready for re-review.

Copy link
Copy Markdown
Contributor

Thanks @noevidence1017 for starting the type-safety work. Backend CI is green for this slice, but since this PR only covers the onchain/soroban portion (1/3), issue #99 won't fully resolve until the rest of the codebase is addressed.

Could you follow up with the remaining 2/3 in separate PRs (or update this one's scope to indicate the subset) before we merge? Holding the merge in the meantime so the issue stays open for the follow-ups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HIGH] Replace any types with proper TypeScript types across codebase

2 participants