Harden Ledger interactive submission end to end#384
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (39)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
CI follow-up on
Validation: |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
CI diagnosis and fix on
Local validation: TypeScript 7 lint build; focused interactive/retry suites 151/151; touched ESLint/Prettier; shell syntax and workflow check. Fresh pinned-version Quickstart CI is running. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Pinned-version follow-up on The first 0.6.8 image run proved the image override worked, then failed before tests because the older Quickstart fixture lacked the 0.6.x Validation: shell syntax, diff checks, and package-artifact gate pass locally. Fresh full LocalNet CI is running. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
The first full 0.6.8 LocalNet run reached all 33 integration suites; 32 passed. The sole failure exposed one more OpenAPI/Scala JSON-boundary mismatch:
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
…ctive-submission-foundation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Upstream-base repair on
Validation: full TypeScript 7 build; focused interactive/retry/external-signing/Scan suites; conflict-file ESLint/Prettier; shell syntax; package-artifact gate; diff checks. Fresh CI is running on the merge-clean base. |
…tegies' into codex/ledger-interactive-submission-foundation
|
Synced the final request-body isolation fix from #383 via normal merge commit |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
cursor review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3875cc5. Configure here.
|
After #383 squash-merged, retargeting this stacked PR to main exposed divergent ancestry and GitHub marked it DIRTY. Resetting to draft/unassigned while I merge current main normally into this branch (no rebase/amend/force), validate the resolved tree, and rerun CI. |
There was a problem hiding this comment.
Pull request overview
This PR hardens Ledger JSON API interactive submission end-to-end by aligning the SDK with the pinned OpenAPI contract, tightening request/response validation, and enforcing safer retry semantics (especially around mutation freshness and response validation outside the transport retry loop).
Changes:
- Adds strict, pinned interactive submission endpoints (
execute,executeAndWait,executeAndWaitForTransaction) with schema validation and fresh-submission-id retry enforcement. - Introduces operation-level execution options (signal + typed retry strategies) and maps them safely onto HTTP retry/failover semantics (including Scan endpoint rotation).
- Updates external-signing and traffic-estimation helpers/tests to use the stricter types (hashing scheme version, required estimation timestamp, non-empty signatures/commands).
Reviewed changes
Copilot reviewed 84 out of 84 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/traffic/get-estimated-traffic-cost.test.ts | Updates expectations for required estimation timestamp. |
| test/unit/traffic/estimate-traffic-cost.test.ts | Adjusts traffic estimation tests for hashing scheme + timestamp. |
| test/unit/operations/custom-api-operation-options.test.ts | Verifies options forwarding through custom operations and clients. |
| test/unit/operations/api-operation-retry.test.ts | Adds extensive retry/semantics/freshness coverage for operations. |
| test/unit/external-signing/prepare-external-transaction.test.ts | Updates prepare helper tests for pinned prepare contract. |
| test/unit/external-signing/external-signing-client.test.ts | Switches orchestration to typed interactive submit operations. |
| test/unit/external-signing/external-party-wallet.test.ts | Updates wallet bridge tests to typed executeAndWait path. |
| test/unit/external-signing/execute-external-transaction.test.ts | Tightens helper inputs (non-empty signatures) + typed responses. |
| test/unit/core/request-value.test.ts | Adds tests for cloning/freezing request snapshots. |
| test/unit/core/errors.test.ts | Adds UnknownMutationOutcomeError tests and revises rejection certainty. |
| test/unit/clients/scan-api.test.ts | Adds coverage for retry+failover behavior across scan endpoints. |
| test/unit/amulet/external-party-transfer-offer.test.ts | Updates transfer-offer helpers to typed executeAndWait behavior. |
| test/typecheck/operation-retry.typecheck.ts | Type-level coverage for retry option typing and constraints. |
| test/typecheck/ledger-interactive-submission.typecheck.ts | Type-level coverage for pinned interactive submission contract. |
| test/integration/localnet/ledger-api/setup.ts | Adds helper to resolve wallet-install context for integration tests. |
| test/integration/localnet/ledger-api/interactive-submission.test.ts | Adds LocalNet E2E tests for all execute variants + payload checks. |
| src/utils/traffic/types.ts | Makes estimatedAt required on traffic cost estimates. |
| src/utils/traffic/get-estimated-traffic-cost.ts | Always returns estimation timestamp as estimatedAt. |
| src/utils/traffic/estimate-traffic-cost.ts | Requires hashing scheme version and threads into prepare call. |
| src/utils/mining/mining-rounds.ts | Threads execute options into mining-round discovery calls. |
| src/utils/external-signing/prepare-external-transaction.ts | Enforces one-command prepare and non-empty actAs; threads hashing scheme. |
| src/utils/external-signing/external-signing-client.ts | Narrows hashing scheme typing and uses prepared hashing scheme directly. |
| src/utils/external-signing/external-party-wallet.ts | Narrows hashing scheme typing for wallet transfer flows. |
| src/utils/external-signing/execute-external-transaction.ts | Enforces non-empty signatures, typed executeAndWait, adds minLedgerTime. |
| src/utils/amulet/external-party-transfer-offer.ts | Narrows hashing scheme typing and uses typed prepare response. |
| src/core/operations/operation-request-options.ts | New: maps operation retry/options to HTTP retry/options safely. |
| src/core/operations/operation-execute-options.ts | New: typed operation execute options + snapshotting. |
| src/core/operations/index.ts | Re-exports new operation execution/option modules. |
| src/core/operations/ApiOperationFactory.ts | Adds request semantics, response schema validation, and retry plumbing. |
| src/core/operations/ApiOperation.ts | Adds response validation + typed HTTP options plumbing. |
| src/core/http/request-value.ts | New: clone/freeze utilities for request snapshot immutability. |
| src/core/http/request-retry.ts | New: typed HTTP retry semantics, hooks, and snapshotting. |
| src/core/http/index.ts | Re-exports request-retry types/utilities. |
| src/core/http/abort.ts | New: stable AbortError creation and abortable pre-dispatch awaits. |
| src/core/errors.ts | Adds UnknownMutationOutcomeError + revises definite rejection semantics. |
| src/core/BaseClient.ts | Threads HTTP retry/semantics options through BaseClient request methods. |
| src/clients/validator-api/operations/v0/wallet/transfer-offers/get-status.ts | Marks POST status lookup as semantic read. |
| src/clients/validator-api/operations/v0/wallet/buy-traffic-requests/get-status.ts | Marks POST status lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/transfer-instruction/v1/get-transfer-instruction-withdraw-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/transfer-instruction/v1/get-transfer-instruction-reject-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/transfer-instruction/v1/get-transfer-instruction-accept-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/transfer-instruction/v1/get-transfer-factory.ts | Marks factory lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/allocations/v1/get-allocation-withdraw-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/allocations/v1/get-allocation-transfer-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/allocations/v1/get-allocation-cancel-context.ts | Marks choice-context lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/registry/allocation-instruction/v1/get-allocation-factory.ts | Marks factory lookup as semantic read. |
| src/clients/validator-api/operations/v0/scan-proxy/get-member-traffic-status.ts | Threads operation options through discovery + final request with stable params. |
| src/clients/validator-api/operations/v0/ans/get-rules.ts | Marks POST rules lookup as semantic read. |
| src/clients/validator-api/operations/v0/admin/prepare-transfer-preapproval-send.ts | Marks prepare endpoint as semantic read. |
| src/clients/validator-api/operations/v0/admin/prepare-accept-external-party-setup-proposal.ts | Marks prepare endpoint as semantic read. |
| src/clients/validator-api/operations/v0/admin/generate-external-party-topology.ts | Marks generation endpoint as semantic read (per API contract). |
| src/clients/scan-api/ScanApiClient.ts | Integrates Scan failover into one retry loop with stable attempt budgets. |
| src/clients/scan-api/operations/v0/scan.ts | Marks multiple Scan POST reads as semantic read. |
| src/clients/scan-api/operations/v0/registry/allocation-instruction/v1/get-allocation-factory-from-registry.ts | Marks registry factory lookup as semantic read. |
| src/clients/ledger-json-api/schemas/wire.ts | New: strict wire-level primitives (Base64, RFC3339, hashes, names). |
| src/clients/ledger-json-api/schemas/operations/interactive-submission.ts | Removes phantom endpoints; tightens preferred package request schemas. |
| src/clients/ledger-json-api/schemas/index.ts | Re-exports new wire schemas. |
| src/clients/ledger-json-api/schemas/api/packages.ts | Tightens preferred package schemas + normalizes nullish preference. |
| src/clients/ledger-json-api/operations/v2/updates/get-update-by-offset.ts | Marks POST query as semantic read. |
| src/clients/ledger-json-api/operations/v2/updates/get-update-by-id.ts | Marks POST query as semantic read. |
| src/clients/ledger-json-api/operations/v2/updates/get-transaction-by-offset.ts | Marks POST query as semantic read. |
| src/clients/ledger-json-api/operations/v2/updates/get-transaction-by-id.ts | Marks POST query as semantic read. |
| src/clients/ledger-json-api/operations/v2/parties/list.ts | Threads execute options through pagination helper. |
| src/clients/ledger-json-api/operations/v2/parties/get.ts | Threads execute options through pagination helper. |
| src/clients/ledger-json-api/operations/v2/parties/external/generate-topology.ts | Marks POST as semantic read. |
| src/clients/ledger-json-api/operations/v2/parties/aggregate-parties.ts | Adds failover/retry options support to party pagination. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/upload-dar.ts | Removes phantom upload-dar operation. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/prepare.ts | Adds response validation; marks as semantic read; normalizes request payload. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/index.ts | Exports new execute variants; removes phantom operations. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/get-preferred-packages.ts | Adds response validation; marks as semantic read; passes strict params. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/get-preferred-package-version.ts | Adds response validation for preferred version lookup. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/execute.ts | Adds response validation; enforces fresh retry identifiers. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/execute-and-wait.ts | New: typed execute-and-wait operation with validation + freshness. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/execute-and-wait-for-transaction.ts | New: typed execute-and-wait-for-transaction with validation + freshness. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/create-user.ts | Removes phantom create-user operation. |
| src/clients/ledger-json-api/operations/v2/interactive-submission/allocate-party.ts | Removes phantom allocate-party operation. |
| src/clients/ledger-json-api/operations/v2/events/get-events-by-contract-id.ts | Marks POST query as semantic read. |
| src/clients/ledger-json-api/operations/v2/commands/completions.ts | Marks POST completions polling as semantic read. |
| scripts/generate-all-client-methods.ts | Uses TS AST to extract class op signatures and option params. |
| .github/workflows/test-cn-quickstart.yml | Pins CN-Quickstart Splice version from libs/splice/VERSION. |
|
Reconciled the branch with the squash-merged main branch via a normal merge commit (514c900) and preserved the interactive-submission changes without rewriting history.\n\nPost-resolution validation:\n- TypeScript 7 core + lint builds passed\n- focused retry/interactive/HTTP/Scan unit tests: 211/211 passed\n- targeted ESLint + Prettier passed\n- package artifact validation passed (6.86 MB, 1,815 files)\n- no conflict markers or whitespace errors\n\nKeeping this PR in draft and unassigned while fresh CI runs on the new head. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
|
cursor review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e48c1ca. Configure here.
|
fix the pr description |
What changed\n\n- add the pinned Ledger JSON API interactive submission endpoints for asynchronous execute, execute-and-wait, and execute-and-wait-for-transaction\n- replace permissive generated shapes with exact request and response schemas for prepare, execute, transaction events, and preferred-package APIs\n- validate canonical Base64 bytes, RFC 3339 timestamps, Daml-LF transaction hashes, protobuf numeric bounds, exact one-of unions, non-empty signatures, and lossless Daml JSON values\n- normalize wire-level nullable option fields to absent TypeScript properties while preserving meaningful Daml JSON null\n- reject unspecified enum sentinels and remove phantom interactive allocate-party, create-user, and upload-DAR operations that are absent from the pinned API\n- validate successful responses outside the transport retry loop so malformed 200 responses are never replayed\n- require fresh submission IDs for every explicit derived mutation retry, reject exact-body replay at type and runtime boundaries, and redact reused identifiers from errors\n- update external-signing, external-wallet, transfer-offer, and traffic-estimation helpers to use the strict endpoint types\n\n## End-to-end coverage\n\n- prepare against the live pinned Quickstart Ledger endpoint and assert the exact normalized response\n- create an externally controlled Ed25519 party, resolve preferred packages, prepare and sign real AppInstall transactions, and execute all three submission variants\n- verify each committed transaction payload and exact 64-character external transaction hash through the Ledger API\n- assert distinct submission IDs and prepared hashes across retry-sensitive executions\n- run the complete pinned LocalNet integration suite: 34 suites, 79 passed, 4 intentionally skipped\n\n## Final validation\n\n- TypeScript 7 core and lint builds passed\n- focused retry, interactive, HTTP, Scan, and traffic tests passed, including the final review fix\n- targeted ESLint and Prettier passed\n- package artifact gate passed at 6.86 MB across 1,815 files\n- final-head GitHub checks passed, including test-cn-quickstart, package artifacts, pinned dependencies, CodeQL, and security checks\n- branch reconciled with current main through normal merge commits; no history rewrite\n\n## Compatibility\n\nThis is intentionally breaking. The SDK exposes the pinned API contract rather than preserving phantom operations or permissive wire types.