Skip to content

fix(sdk-coin-avaxp): prevent credential wipe and expose incomplete signing state#9287

Draft
nvrakesh06 wants to merge 1 commit into
masterfrom
fix/avaxp-credential-guard-bypass
Draft

fix(sdk-coin-avaxp): prevent credential wipe and expose incomplete signing state#9287
nvrakesh06 wants to merge 1 commit into
masterfrom
fix/avaxp-credential-guard-bypass

Conversation

@nvrakesh06

Copy link
Copy Markdown
Contributor

Summary

  • hasCredentials now checks != null only (was !== undefined && length > 0). credentials=[] previously bypassed the guard in buildAvaxTransaction(), causing it to regenerate fresh placeholder credentials and wipe any signature already placed by a prior signer.
  • sign() adds an explicit length === 0 guard to compensate — empty credentials still throw an appropriate error.
  • signature getter now collects real ECDSAs across all credentials instead of only credentials[0], making an incompletely-signed tx detectable via toJson().signatures before it reaches the sendq.

Background

Production incident CECHO-1697 (2026-07-16): a custodial 2-of-3 AVAXP ExportTx P→C was broadcast with sig[1] still containing an address placeholder (r=0) in every credential, causing AvalancheGo to reject with:

failed verifySpend: failed to verify transfer: invalid signature

Tx: 8xbiLpsKDKkJrN3YUqcZpFcxApLCwmQkpKUuvmHU1GL9RmAyu

Root cause: if credentials=[] arises during BitGo Signer 2's deserialization of the half-signed tx, hasCredentials returned false, buildAvaxTransaction() regenerated fresh placeholders (wiping Signer 1's ECDSA), and Signer 2 signed into the wrong slot — leaving slot[1] as address placeholder with r=0.

A retry the next day with a different signer succeeded on the same unsigned tx, confirming the bug is in the credential guard path, not the UTXOs or policy.

Proven by test/diag/diag3-guard-bypass.ts (before fix: credentials regenerated; after fix: credentials preserved).

Note: The definitive fix for the production failure path requires a wallet-platform change to validate all credential slots have r≠0 before inserting into the sendq. These SDK changes are a hardening layer that catches the bug earlier if the WP server routes through the SDK signing path.

Test plan

  • Run test/diag/diag3-guard-bypass.ts — confirm hasCredentials=true for credentials=[] after fix
  • Run test/diag/diag4-reproduce-invalid-signature.ts — confirm Phase 3 (correct path) still produces 2 valid ECDSAs
  • Run existing sdk-coin-avaxp unit tests
  • Enable and pass the skipped test TODO(BG-56700) in test/unit/lib/exportC2PTxBuilder.ts

🤖 Generated with Claude Code

…gning state

hasCredentials was checking credentials.length > 0, which returns false for
credentials=[] (empty array). This allowed buildAvaxTransaction() to regenerate
fresh placeholder credentials during HSM signing, wiping any partial signature
already placed by a prior signer. The regenerated credentials left the second
signature slot as an address placeholder (r=0), causing AvalancheGo to reject
the tx with "failed verifySpend: invalid signature".

Three fixes:
- hasCredentials now checks != null only, so credentials=[] blocks credential
  regeneration rather than triggering it
- sign() adds an explicit length guard so empty credentials still throw
- signature getter now collects real ECDSAs across all credentials instead of
  only credentials[0], making incompletely-signed txs visible before broadcast

Relates to CECHO-1697. Production failure on ExportTx P->C (custodial 2-of-3,
tx 8xbiLpsKDKkJrN3YUqcZpFcxApLCwmQkpKUuvmHU1GL9RmAyu, 2026-07-16).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant