-
Notifications
You must be signed in to change notification settings - Fork 0
Enforce strict typing for mining rounds #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ility - Make OpenMiningRoundSchema payload fields required (opensAt, round_number) - Use canonical snake_case format for mining round fields - Remove fallback chains for round_number (??roundNumber??round.number) - Make IssuingMiningRoundSchema contract_id required, remove nested contract fallback - Update mining-rounds utility to use direct field access - Update unit tests to match strict schema format - Also includes prior strict typing changes for amulet/contract utilities
|
Claude finished @HardlyDifficult's task —— View job Code Review CompletedI've thoroughly reviewed this PR that enforces strict typing for mining rounds. Here's my analysis: Todo List
SummaryThis is a well-executed cleanup PR that removes backwards compatibility fallbacks and enforces canonical snake_case formatting for mining round schemas. The changes are consistent across the codebase and properly tested. IssuesNone identified. The code follows consistent patterns and defensive programming practices. Full AnalysisPositive AspectsClean Schema Simplification - The removal of fallback chains ( Consistent Naming Convention - Enforcing snake_case format across all mining round APIs creates better consistency with the Validator API conventions. Comprehensive Test Updates - All tests have been properly updated to use the canonical format, with good coverage of edge cases like malformed data and missing fields. Defensive Runtime Validation - The utility functions still include proper error handling for malformed data at runtime, even with strict schemas:
Type Safety Improvements - Making Code Quality AssessmentSchema Design (
Utility Function Refactoring
Test Coverage
Performance & SecurityPerformance: The removal of fallback logic actually improves performance by eliminating unnecessary property lookups. Security: No security concerns identified. The changes maintain proper input validation and error handling. Breaking Changes AssessmentThis is an intentional breaking change that removes backwards compatibility. However:
Recommendation: ✅ Approve - This is a clean, well-tested refactoring that improves code maintainability and enforces consistent data formatting. The removal of backwards compatibility is intentional and properly documented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR tightens the typing and canonical formats for mining round and contract-related utilities by enforcing snake_case Validator API schemas and removing support for legacy/variant response shapes. It simplifies consumers to rely on a single canonical shape while keeping runtime behavior defensive where needed.
Changes:
- Enforces canonical snake_case mining-round response schemas in
scan-proxy.tsand removes fallback chains for alternative field locations. - Updates mining-round and amulet utilities (
mining-rounds.ts,get-amulets-for-transfer.ts,findCreatedEvent.ts) to use strict, canonical shapes only. - Aligns unit tests with the new canonical formats and drops tests that covered legacy/nested structures.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/clients/validator-api/schemas/api/scan-proxy.ts | Makes mining round Zod schemas strict to the canonical snake_case fields (round_number, contract_id, required opensAt) that upstream utilities now depend on. |
| src/utils/mining/mining-rounds.ts | Simplifies round-number and contract-id extraction to rely solely on the canonical Validator API schema, and keeps opensAt filtering defensive against malformed timestamps. |
| src/utils/contracts/findCreatedEvent.ts | Aligns CreatedTreeEventValue with the canonical Ledger JSON API event schema and restricts findCreatedEventByTemplateId to the documented transactionTree.eventsById layout. |
| src/utils/amulet/get-amulets-for-transfer.ts | Removes legacy contract shape handling, adds focused helpers for amount/owner extraction, and restricts processing to canonical JsActiveContract responses. |
| test/unit/mining/mining-rounds.test.ts | Updates mining-round tests to use round_number in payloads, treats malformed opensAt as filtered-out, and removes tests for deprecated round-number formats. |
| test/unit/contracts/findCreatedEvent.test.ts | Drops coverage for the no-longer-supported nested transaction.transactionTree.eventsById shape and keeps tests aligned with the canonical transactionTree.eventsById. |
| test/unit/amulet/get-amulets-for-transfer.test.ts | Updates helper contracts and expectations to only use the canonical JsActiveContract shape and validates the new amount/owner extraction and sorting behavior (including coupons and numeric amounts). |
Summary
??patterns) for round_number and contract_idChanges
Schemas (
scan-proxy.ts)OpenMiningRoundSchema: MadeopensAtandround_numberrequired, removedroundNumberand nestedround.numberalternativesIssuingMiningRoundSchema: Madecontract_idrequired, removed nestedcontract.contract_idfallbackUtility Functions
mining-rounds.ts: Removed fallback chains, use direct field accessget-amulets-for-transfer.ts: Strict contract format (from prior session)findCreatedEvent.ts: Removed nested structure fallbacks (from prior session)Tests
Test plan
resolves ENG-513
Note
Medium Risk
Moderate risk because it removes backward-compatibility paths; any upstream services still sending legacy/variant shapes will now fail validation or be ignored, potentially breaking mining-round and transfer flows.
Overview
Enforces the Validator API’s canonical snake_case mining-round response shape by making
OpenMiningRoundSchema.payload.opensAt/round_numberandIssuingMiningRoundSchema.contract_idrequired, and removing support for alternate/legacy field locations.Updates mining-round utilities to use direct field access (no fallback chains) and adjusts unit tests accordingly, dropping coverage for legacy formats. Related cleanup tightens ledger contract parsing in
getAmuletsForTransferandfindCreatedEventByTemplateIdto only accept the canonical JSON API structures.Written by Cursor Bugbot for commit c149926. This will update automatically on new commits. Configure here.