feat: add SDK transaction build validation pipeline (#249)

.github/checklists/issue-249.md

@@ -0,0 +1,28 @@
+# Acceptance Criteria Checklist — Issue #249
+
+> Generated for pre-PR verification. Confirm each item, then run:
+>
+> ```bash
+> npm run verify:pr -- --checklist .github/checklists/issue-249.md
+> ```
+
+## Issue
+
+- **Number:** #249
+- **Title:** Add SDK transaction build validation pipeline
+
+## Acceptance Criteria
+
+- [x] Reusable transaction validators are added
+- [x] Validation order is documented
+- [x] Typed errors are returned
+- [x] Tests cover invalid inputs
+- [x] Existing flows use validators where practical
+
+## Contributor confirmations
+
+- [x] Automated checks passed (`npm run verify:pr`)
+- [x] Tests added or updated for behaviour changes
+- [x] Documentation updated when public behaviour changed
+- [x] PR description maps each acceptance criterion to the change
+- [x] No secrets or `.env` values committed

docs/transaction-build-validation.md

@@ -0,0 +1,117 @@
+# Transaction build validation pipeline
+
+A reusable pipeline that validates the inputs a transaction is built from, in a
+documented order, before anything is built or signed.
+
+```ts
+import { validateTransactionBuild } from 'stellar-pocketpay-sdk';
+
+const result = validateTransactionBuild({ sourceSecret, destination, amount, memo });
+if (!result.valid) {
+  for (const issue of result.issues) showFieldError(issue.field, issue.message);
+  return;
+}
+```
+
+## Why it exists
+
+The duplication was literal. `sendXLM` hand-chained four validators, and
+`sendAsset` repeated the same four with `validateAssetSpec` appended. Both now
+call the pipeline instead.
+
+A non-throwing family already existed that the build path never used — but the
+helpers could not simply be called in sequence, because they return **four
+different shapes**:
+
+| Helper | Returns |
+|---|---|
+| `safeValidateMemo` | `{ valid } \| { valid, error }` |
+| `safeParseAmount` | `{ valid, amount } \| { valid, error }` |
+| `safeValidateDestination` | `Promise<PocketPayResult<…>>` |
+| `validateSendXLMParams` | `{ ok } \| { ok, errors: ValidationError[] }` |
+
+Normalising those into one result is what the pipeline does.
+
+## Validation order
+
+`VALIDATION_ORDER` is exported so the order is checkable, not just documented.
+Local, zero-cost checks run first; anything that reads configuration or an
+account runs last, so a malformed key is reported without resolving config at
+all.
+
+| # | Stage | Reads | Notes |
+|---|---|---|---|
+| 1 | `sourceAccount` | — | The only stage that touches the secret |
+| 2 | `destination` | — | Includes self-payment, which needs the derived source |
+| 3 | `amount` | — | Format, positivity, 7-decimal precision |
+| 4 | `asset` | — | Issued-asset spec shape |
+| 5 | `memo` | — | Type and byte length |
+| 6 | `network` | SDK config | Validates URLs, timeout, contract ID |
+| 7 | `signerCapability` | account | See the limitation below |
+
+### Issues accumulate
+
+Stages are **not** short-circuited. Three malformed fields produce three issues,
+so a form can show all of them at once instead of one per submit.
+`assertTransactionBuildValid` is the throwing variant for call sites that want
+the first failure only.
+
+### Running a subset
+
+```ts
+validateTransactionBuild(input, { stages: ['sourceAccount', 'amount'] });
+```
+
+`sendXLM` and `sendAsset` use this to skip the `network` stage, because they
+resolve configuration themselves. Skipping it keeps *when* a configuration error
+surfaces exactly where it was before the pipeline existed.
+
+## Error codes are transported, never merged
+
+An issue carries the code the originating validator already produced:
+
+```ts
+{ stage: 'amount', code: 'INVALID_AMOUNT', field: 'amount', message: '…' }
+```
+
+`ValidationErrorCode` (`src/payments/validation.ts`) and the published error
+registry are **separate taxonomies on purpose**. Both appear in the same
+`issues` array unchanged; the pipeline does not unify them.
+
+This is also why the amount stage uses `validateAmount` rather than
+`safeParseAmount`. The safe parser belongs to the safe-amount model and raises
+that model's codes, while the payment surface publishes `INVALID_AMOUNT` and
+`INVALID_AMOUNT_PRECISION`. Composing the safe parser would have been a silent
+breaking change to published codes.
+
+## Adoption is not a breaking change
+
+`assertTransactionBuildValid` throws the **same error object** the underlying
+validator produced. Every published code — `INVALID_SECRET_KEY`,
+`SELF_PAYMENT`, `INVALID_AMOUNT`, `INVALID_AMOUNT_PRECISION` and the asset codes
+— is unchanged by routing through the pipeline, and tests assert exactly that.
+
+The self-payment *message* differs between flows ("Cannot send XLM to yourself"
+vs "Cannot send asset to yourself"). The check is shared; only the sentence is
+passed in via `selfPaymentMessage`.
+
+## Limitation: the signer-capability stage
+
+`mapAuthRequirements` takes a **built** transaction, so full threshold analysis
+cannot run before a transaction exists. The pipeline's `signerCapability` stage
+validates what is knowable pre-build: whether the supplied account declares
+signing capability.
+
+Full authorisation mapping — thresholds, signer weights, unsupported operations
+— remains a post-build step via `mapAuthRequirements` and
+`assertAuthFullyMapped`. The stage is skipped entirely when no `account` is
+supplied.
+
+## API
+
+| Export | Purpose |
+|---|---|
+| `validateTransactionBuild(input, options?)` | Run the pipeline, collect all issues |
+| `assertTransactionBuildValid(input, options?)` | Throw the first failure, unchanged |
+| `VALIDATION_ORDER` | The documented stage order |
+| `ValidationStage`, `TransactionValidationIssue`, `TransactionValidationResult`, `TransactionBuildInput`, `TransactionValidationOptions` | Types |

src/index.ts

@@ -214,6 +214,21 @@ export type {
   LifecycleFailure,
 } from './types';
 
+// ─── Transaction build validation pipeline (issue #249) ─────────────────────
+export {
+  validateTransactionBuild,
+  assertTransactionBuildValid,
+  VALIDATION_ORDER,
+} from './transactions';
+
+export type {
+  ValidationStage,
+  TransactionValidationIssue,
+  TransactionValidationResult,
+  TransactionBuildInput,
+  TransactionValidationOptions,
+} from './transactions';
+
 // ─── Transactions ───────────────────────────────────────────────────────────
 export {
   getTransactions,

src/payments/index.ts

@@ -6,10 +6,11 @@
 import * as StellarSDK from '@stellar/stellar-sdk';
 import { getHorizonServer, getNetworkPassphrase, resolveConfig } from '../config';
 import { SendXLMParams, SendAssetParams, PaymentResult, PocketPayError, SDKConfig, PocketPayResult, EnhancedPocketPayResult } from '../types';
-import { validateSecretKey, validatePublicKey, validateAmount, validateMemoInput, buildMemo, wrapError, toResult, toEnhancedSuccessResult, toEnhancedFailureResult, toEnhancedResult } from '../utils';
+import { buildMemo, wrapError, toResult, toEnhancedSuccessResult, toEnhancedFailureResult, toEnhancedResult } from '../utils';
 import type { ResultWarning, RecoveryHint } from '../errors';
 import { withTimeout } from '../network';
 import { submitWithGuard } from '../transactions/guarded-submit';
+import { assertTransactionBuildValid } from '../transactions/build-validation';
 import { validateAssetSpec, verifyPaymentTrustlineOrThrow } from './trustline';
 
 /**
@@ -31,21 +32,21 @@ export async function sendXLM(
 ): Promise<PaymentResult> {
   const { sourceSecret, destination, amount, memo } = params;
   // ─── Preflight validation (before any network call) ─────────────────────────
-  validateSecretKey(sourceSecret);
-  validatePublicKey(destination);
-  validateAmount(amount);
-  validateMemoInput(memo);
+  // Runs through the shared build-validation pipeline (issue #249) rather than
+  // hand-chaining the same four validators here and again in `sendAsset`. The
+  // pipeline rethrows the originating PocketPayError untouched, so every
+  // published code below is unchanged. `network` is excluded because this
+  // function resolves config itself inside the try block, and moving that would
+  // change when a configuration error surfaces relative to the handler below.
+  assertTransactionBuildValid(
+    { sourceSecret, destination, amount, memo },
+    {
+      stages: ['sourceAccount', 'destination', 'amount', 'memo'],
+      selfPaymentMessage: 'Cannot send XLM to yourself',
+    },
+  );
   const sourceKeypair = StellarSDK.Keypair.fromSecret(sourceSecret);
   const sourcePublic = sourceKeypair.publicKey();
-  if (sourcePublic === destination) {
-    throw new PocketPayError('Cannot send XLM to yourself', 'SELF_PAYMENT', {
-      validation: {
-        field: 'destination',
-        reason: 'same_as_source',
-        value: destination
-      }
-    });
-  }
   try {
     const cfg = resolveConfig(config);
     const server = getHorizonServer(config);
@@ -326,25 +327,21 @@ export async function sendAsset(
   const { sourceSecret, destination, amount, asset, memo, skipTrustlineCheck } = params;
 
   // ─── Preflight validation (synchronous, no network) ──────────────────────
-  validateSecretKey(sourceSecret);
-  validatePublicKey(destination);
-  validateAmount(amount);
-  validateMemoInput(memo);
-  validateAssetSpec(asset);
+  // Same shared pipeline as `sendXLM` (issue #249). This path is where the
+  // duplication was most visible: the identical four validators, plus the asset
+  // check. Only the self-payment wording differs between the two flows, so that
+  // sentence is passed in rather than the check being repeated.
+  assertTransactionBuildValid(
+    { sourceSecret, destination, amount, asset, memo },
+    {
+      stages: ['sourceAccount', 'destination', 'amount', 'asset', 'memo'],
+      selfPaymentMessage: 'Cannot send asset to yourself',
+    },
+  );
 
   const sourceKeypair = StellarSDK.Keypair.fromSecret(sourceSecret);
   const sourcePublic = sourceKeypair.publicKey();
 
-  if (sourcePublic === destination) {
-    throw new PocketPayError('Cannot send asset to yourself', 'SELF_PAYMENT', {
-      validation: {
-        field: 'destination',
-        reason: 'same_as_source',
-        value: destination,
-      },
-    });
-  }
-
   const isNative =
     asset.code.toUpperCase() === 'XLM' || asset.code.toLowerCase() === 'native';