Skip to content

feat(sdk-core): add transaction validation for (consolidate) unspent management #7020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Sep 26, 2025
Merged

feat(sdk-core): add transaction validation for (consolidate) unspent management #7020

merged 8 commits into from
Sep 26, 2025

Conversation

@danielpeng1
Copy link
Contributor

@danielpeng1 danielpeng1 commented Sep 18, 2025 •
edited
Loading

Description:

  • Reordered + refactored transaction prebuild validation to occur after keychain retrieval, ensuring verifyTransaction has access to required keychain data for proper spoofing protection (for both consolidate and fanout)
  • Added unit test for consolidate unspent management spoofability protection in test/btc.ts (for UTXO coins)
  • Verifies that platform responses are validated against user parameters and covers BUILD_SIGN_SEND flow

Ticket: WP-5945

@danielpeng1 danielpeng1 self-assigned this Sep 18, 2025
@danielpeng1 danielpeng1 requested review from a team as code owners September 18, 2025 16:00
kamleshmugdiya

This comment was marked as duplicate.

Sign in to view

kamleshmugdiya
kamleshmugdiya requested changes Sep 18, 2025
View reviewed changes
Copy link
Contributor

@kamleshmugdiya kamleshmugdiya left a comment

Choose a reason for hiding this comment

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

can do this in parallel using Promise.all

@danielpeng1 danielpeng1 force-pushed the WP-5945/sdk-unspent-management-validation branch from 8e9949b to 7300cc0 Compare September 18, 2025 16:51
@danielpeng1 danielpeng1 requested a review from a team as a code owner September 18, 2025 20:22
nvjsr
nvjsr previously approved these changes Sep 19, 2025
View reviewed changes
Copy link
Contributor

@nvjsr nvjsr left a comment

Choose a reason for hiding this comment

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

LGTM

mohammadalfaiyazbitgo
mohammadalfaiyazbitgo requested changes Sep 19, 2025
View reviewed changes
@SimonVutovB
feat(sdk-coin-btc): add transaction validation for fanout unspent man…
a1726a6 
…agement

- Add test case for fanout spoofing detection in btc.ts

Ticket WP-5955

TICKET: WP-5955
@SimonVutovB
refactor(sdk-coin-btc): move require to top
958eb5a 
move require getDefaultWalletKeys, toKeychainObjects to top

TICKET: WP-5955
OttoAllmendinger
OttoAllmendinger requested changes Sep 22, 2025
View reviewed changes
lokesh-bitgo
lokesh-bitgo approved these changes Sep 24, 2025
View reviewed changes
Copy link
Contributor

@lokesh-bitgo lokesh-bitgo left a comment

Choose a reason for hiding this comment

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

Looks good

@danielpeng1 danielpeng1 merged commit e190bc0 into master Sep 26, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OttoAllmendinger OttoAllmendinger OttoAllmendinger approved these changes

@mohammadalfaiyazbitgo mohammadalfaiyazbitgo mohammadalfaiyazbitgo approved these changes

@kamleshmugdiya kamleshmugdiya kamleshmugdiya approved these changes

@lokesh-bitgo lokesh-bitgo lokesh-bitgo approved these changes

@adityasoni-bitgo adityasoni-bitgo Awaiting requested review from adityasoni-bitgo adityasoni-bitgo is a code owner automatically assigned from BitGo/wallet-platform

@nvjsr nvjsr Awaiting requested review from nvjsr

Assignees

@danielpeng1 danielpeng1

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@danielpeng1 @OttoAllmendinger @mohammadalfaiyazbitgo @kamleshmugdiya @lokesh-bitgo @nvjsr @SimonVutovB