Skip to content

fix: align charge module with draft-stellar-charge-00 spec#25

Merged
marcelosalloum merged 5 commits intomainfrom
fix/spec-compliance-rebased
Mar 27, 2026
Merged

fix: align charge module with draft-stellar-charge-00 spec#25
marcelosalloum merged 5 commits intomainfrom
fix/spec-compliance-rebased

Conversation

@marcelosalloum
Copy link
Copy Markdown
Collaborator

@marcelosalloum marcelosalloum commented Mar 27, 2026

What

  • Switch network identifiers to CAIP-2 format (stellar:testnet / stellar:pubnet) in methodDetails.network and credential source field; add CAIP2_NETWORK / CAIP2_TO_NETWORK constants exported from root index
  • Remove reference and memo fields from methodDetails schema
  • Move source (DID-PKH) from inside payload to the top-level credential field
  • Add SettlementError (exported) to distinguish broadcast/confirmation failures from verification errors
  • Add server-side validation for spec gaps:
    • Exactly one invokeHostFunction operation
    • Auth entries must not contain sub-invocations
    • Auth entry expiration must not exceed challenge.expires
    • Server address must not appear in auth entries or simulation transfer events
    • timeBounds.maxTime must not exceed challenge.expires on unsponsored path
    • Pre-submission simulation with CAP-46 transfer event validation
    • SettlementError for broadcast and confirmation failures
  • Derive ledger expiration from challenge.expires instead of a fixed timeout
  • Add DEFAULT_LEDGER_CLOSE_TIME and DEFAULT_CHALLENGE_EXPIRY constants

Why

The charge module had several gaps relative to draft-stellar-charge-00. This PR closes them to improve interoperability and security: CAIP-2 is the spec's standard chain reference format; the DID-PKH source must be a top-level credential field (the mppx Credential type already modeled this correctly but the client was placing it inside payload); the server must validate transaction structure and auth entries before broadcasting to prevent abuse.

API changes (before → after)

methodDetails.network — CAIP-2 format

- { network: 'testnet' }
+ { network: 'stellar:testnet' }

Credential source field — moved to top level

- Credential.serialize({ challenge, payload: { type: 'transaction', transaction: xdr, source } })
+ Credential.serialize({ challenge, payload: { type: 'transaction', transaction: xdr }, source })

New error class

import { SettlementError } from 'stellar-mpp-sdk'
// Thrown when broadcast or confirmation fails (distinct from PaymentVerificationError)

New constants

import { CAIP2_NETWORK, CAIP2_TO_NETWORK, DEFAULT_LEDGER_CLOSE_TIME } from 'stellar-mpp-sdk'
CAIP2_NETWORK['testnet']           // → 'stellar:testnet'
CAIP2_TO_NETWORK['stellar:pubnet'] // → 'public'

Copilot AI review requested due to automatic review settings March 27, 2026 21:00
@marcelosalloum marcelosalloum self-assigned this Mar 27, 2026
@marcelosalloum marcelosalloum changed the title fix: align charge credential structure with draft-stellar-charge-00 spec fix: align charge module with draft-stellar-charge-00 spec Mar 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Aligns the Stellar charge method’s credential and request structures with the draft-stellar-charge-00 spec and related mppx conventions, while extending server-side verification/settlement behavior.

Changes:

  • Move toward spec-compliant credential/request fields (top-level source, CAIP-2 network identifiers).
  • Add new constants and public exports (CAIP-2 mappings, ledger timing defaults, SettlementError).
  • Expand test coverage across unit + integration tests for the updated structures and mappings.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
sdk/src/shared/errors.ts Adds SettlementError error type for settlement/broadcast/confirmation failures.
sdk/src/index.ts Re-exports CAIP-2 + default constants and exposes SettlementError publicly.
sdk/src/constants.ts Introduces CAIP-2 network mapping constants and ledger/challenge timing defaults.
sdk/src/charge/server/Charge.ts Updates request transform to CAIP-2, adds settlement error typing, and adds simulation/auth/timebounds validations.
sdk/src/charge/server/Charge.test.ts Adds tests for request() transform (CAIP-2 network + feePayer behavior) and updates hash-credential setup.
sdk/src/charge/integration.test.ts Updates mocked challenge network to CAIP-2 and adds spec-compliance tests for credential structure/source field placement.
sdk/src/charge/client/Charge.ts Updates client network parsing to CAIP-2, adds DID-PKH source to credentials, and attempts to derive timebounds from challenge.expires.
sdk/src/charge/client/Charge.test.ts Adds tests for missing key inputs, CAIP-2 mapping, and DID-PKH formatting.
sdk/src/charge/Methods.ts Updates request schema methodDetails to require CAIP-2 network (and optional feePayer).
sdk/src/charge/Methods.test.ts Adjusts schema tests for CAIP-2 network and refines payload discriminated union assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Type CAIP2_TO_NETWORK as Record<string, NetworkId | undefined> to accurately
  reflect that unknown CAIP-2 keys return undefined (#1)
- Replace (builder as any).timeBounds mutation with setTimebounds() public API;
  the internal property is `timebounds` (lowercase) so the cast was a no-op,
  causing setTimeout(0) to set maxTime=now and immediately expire sponsored
  and unsponsored transactions (#2, #3)
- Derive DID-PKH network component from resolved NetworkId, not raw CAIP-2
  string, so source stays consistent when unknown identifiers fall back (#6, #7)
- Replace inline Promise.race simulation with shared simulateCall() to get
  proper timer cleanup; map SimulationContractError → PaymentVerificationError (#4)
@marcelosalloum marcelosalloum force-pushed the fix/spec-compliance-rebased branch from d4480ad to eb38823 Compare March 27, 2026 21:36
@marcelosalloum marcelosalloum merged commit 1bbae94 into main Mar 27, 2026
5 checks passed
@marcelosalloum marcelosalloum deleted the fix/spec-compliance-rebased branch March 27, 2026 21:40
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.

2 participants