[Ranjna-G]

Summary

Wallet.sweep now supplies the wallet pub triple when explaining the /sweepWallet prebuild, fixing pub triple is required failures that broke every UTXO sweep (BTC, BCH, LTC, DOGE, …) on bitgo-express >= 16.0.0.

Linear: COINS-1797

Changes

• Hoist the keychain fetch that sweep already performed for signing above the explain call (modules/sdk-core/src/bitgo/wallet/wallet.ts)
• Pass pubs to explainTransaction: the wasm-utxo explain requires the wallet pub triple to classify change outputs, and the /sweepWallet prebuild response carries no pubs — sweep previously passed neither pubs nor wallet
• Pass pubs into the signing params so Wallet.signTransaction skips its internal keychain re-fetch — total keychain requests unchanged (3 GETs per sweep)
• Remove a pre-existing as any (typed as Keychain[])
• Add modules/abstract-utxo/test/unit/sweep.ts: end-to-end sweep regression test (prebuild → explain → destination validation → sign → send, asserting exactly one keychain fetch) plus a destination-mismatch guard test

Test Plan

• cd modules/abstract-utxo && npx mocha test/unit/sweep.ts → 2 passing; verified red with pub triple is required when the fix is reverted and sdk-core dist rebuilt
• Full abstract-utxo unit suite: 1666 passing, 5 pending, 0 failing
• sdk-core suite failures are pre-existing dependency drift (@bitgo/public-types KeyBulkUpdate*, @bitgo/sdk-lib-mpc MpsDerive), verified identical with the fix reverted

Closes COINS-1797

[linear-code]

COINS-1797

[Ranjna-G]

@claude review

[github-actions]

Claude finished @Ranjna-G's task —— View job

---

Code Review Complete ✅

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze git diff to understand exact changes
• Review wallet.ts modifications
• Review new sweep.ts test file
• Check code quality, security, and best practices
• Provide comprehensive review feedback

---

Code Review Summary

This PR successfully addresses the "pub triple is required" error in UTXO wallet sweeps by providing the necessary wallet public keys to the explainTransaction call. The implementation is well-architected and correctly solves the problem.

✅ Strengths

Problem Resolution

• Root cause correctly identified: The /sweepWallet API endpoint doesn't include pubs in its response, but explainTransaction needs the wallet public keys to classify change outputs vs external outputs
• Elegant solution: Hoisting the keychain fetch that was already needed for signing to also serve the explain operation
• Performance neutral: Total keychain requests remain unchanged (3 GETs per sweep)

Code Quality

• Good separation of concerns: The fix is localized to the sweep method without affecting other wallet operations
• Type safety improvement: Removes a pre-existing as any cast and properly types keychains as Keychain[]
• Clear documentation: Excellent inline comments explaining why the pub triple is required

Testing

• Comprehensive test coverage: New test file covers both the happy path and error cases
• End-to-end validation: Tests the entire flow from prebuild → explain → validation → sign → send
• Performance verification: Asserts exactly one keychain fetch occurs (validates the optimization)
• Edge case coverage: Tests destination address validation

🔍 Technical Analysis

wallet.ts Changes (modules/sdk-core/src/bitgo/wallet/wallet.ts:1170-1193)

// BEFORE: explainTransaction called without pubs
const transaction = await this.baseCoin.explainTransaction(response);

// AFTER: keychains fetched early and pubs provided
const keychains: Keychain[] = await this.baseCoin.keychains().getKeysForSigning({ wallet: this, reqId });
const pubs = keychains.map((keychain) => {
  assert(keychain.pub);
  return keychain.pub;
});
const transaction = await this.baseCoin.explainTransaction({ ...response, pubs });

Analysis:

• ✅ The assert(keychain.pub) ensures type safety and prevents runtime errors
• ✅ Proper spread operator usage to merge response with pubs
• ✅ Reuses fetched keychains for signing (line 1190: pubs,)

Test Implementation (modules/abstract-utxo/test/unit/sweep.ts)

The test file demonstrates strong testing practices:

• ✅ Realistic test data: Uses proper PSBT construction with appropriate script types
• ✅ Mock validation: Each test verifies all HTTP calls were made exactly once via scope.isDone()
• ✅ Error path testing: Second test validates rejection when destination doesn't match prebuild outputs
• ✅ Isolation: Proper cleanup with nock.cleanAll() in afterEach

💡 Minor Observations

1. Code organization: The keychain extraction logic could potentially be extracted to a helper method if this pattern is needed elsewhere, but it's fine as-is for a single use case.

2. Error messaging: The error message format invalid sweep destination ${invalidOutputAddress.address}, specified ${params.address} is clear and actionable.

3. Type consistency: The code properly handles the case where keychains might have different lengths (backup/bitgo keychains may be optional).

🚀 Recommendation

APPROVE - This is a well-implemented fix that:

• ✅ Solves the reported issue correctly
• ✅ Maintains backward compatibility
• ✅ Includes comprehensive tests
• ✅ Follows repository coding standards
• ✅ Has no security concerns
• ✅ Improves code quality (removes as any)

The implementation correctly addresses the wasm-utxo explain requirement for wallet public keys and should resolve the sweep failures across all UTXO coins (BTC, BCH, LTC, DOGE) on bitgo-express >= 16.0.0.

---

Job run: https://github.com/BitGo/BitGoJS/actions/runs/

[github-actions]

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run