Skip to content

feat(btc): expose coinselect sortingStrategy for deterministic output ordering, OK-50996#10493

Merged
originalix merged 6 commits into
xfrom
feat/btc-coinselect-sorting-strategy
Mar 5, 2026
Merged

feat(btc): expose coinselect sortingStrategy for deterministic output ordering, OK-50996#10493
originalix merged 6 commits into
xfrom
feat/btc-coinselect-sorting-strategy

Conversation

@originalix

@originalix originalix commented Mar 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Expose sortingStrategy parameter through coinSelectWithWitness to control BTC transaction output ordering
  • For swap/protocol transactions (detected by opReturn presence), use 'none' strategy to guarantee payment address as the first output
  • Regular transfers continue using default 'random' strategy for privacy

Intent & Context

Third-party protocols (e.g., THORSwap) require the to address to appear as the first output in BTC transactions. The @onekeyfe/coinselect SDK uses a 'random' sorting strategy by default, which randomly inserts the change output before or after the payment output (~50% chance). This breaks protocol requirements.

The coinselect SDK was updated in OneKeyHQ/coinselect#17 to accept an optional sortingStrategy parameter. This PR wires that parameter through the app-monorepo call chain.

Design Decisions

  • Detection via opReturn: Presence of opReturn in transfersInfo reliably indicates a protocol transaction (THORSwap, etc.), not a regular user transfer. This avoids adding new fields to ITransferInfo.
  • Scoped impact: Only swap/protocol transactions are affected. Regular BTC transfers keep 'random' sorting for privacy (prevents observers from identifying payment vs change outputs by position).
  • undefined as default: When no sortingStrategy is passed, the SDK defaults to 'random', maintaining full backward compatibility.

Changes Detail

  • packages/core/src/utils/coinSelectUtils.ts: Added ICoinSelectSortingStrategy type; added sortingStrategy to ICoinSelectWithWitnessOptions and passed it through to the SDK
  • packages/kit-bg/src/vaults/impls/btc/Vault.ts: Pass sortingStrategy: 'none' when opReturn is present in transfer info

Risk Assessment

  • Risk Level: Low
  • Affected Platforms: All (Extension / Mobile / Desktop / Web)
  • Risk Areas: Only affects BTC transactions that go through local coin selection with opReturn (THORSwap-style swaps). Regular transfers and PSBT-based swaps are unaffected.

Test plan

  • Verify regular BTC transfers still work with randomized output ordering
  • Verify BTC swap transactions with opReturn produce payment address as first output
  • Verify PSBT-based swaps (non-THORSwap) are unaffected

Open with Devin

originalix and others added 3 commits March 5, 2026 10:47
Document the plan to expose sortingStrategy parameter in
@onekeyfe/coinselect witness.js for deterministic output ordering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nsactions

Expose sortingStrategy parameter through coinSelectWithWitness to control
BTC transaction output ordering. When opReturn is present (protocol
transactions like THORSwap), use 'none' strategy to guarantee the payment
address appears as the first output. Regular transfers continue using the
default 'random' strategy for privacy.
@revan-zhang

revan-zhang commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 53f7835c05

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/kit-bg/src/vaults/impls/btc/Vault.ts
devin-ai-integration[bot]

This comment was marked as resolved.

@socket-security

socket-security Bot commented Mar 5, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Added@​babel/​plugin-proposal-class-properties@​7.18.61001006550100
Added@​babel/​plugin-proposal-class-static-block@​7.21.01001007150100
Added@​babel/​plugin-proposal-nullish-coalescing-operator@​7.18.61001007050100
Added@​babel/​plugin-proposal-private-methods@​7.18.61001006550100
Added@​babel/​plugin-proposal-private-property-in-object@​7.21.111001007350100
Added@​aptos-labs/​ts-sdk@​1.39.09710010050100
Added@​babel/​plugin-transform-numeric-separator@​7.25.91001006794100
Added@​expo/​plist@​0.1.3741007199100
Added@​types/​node-fetch@​2.6.91001007281100
Added@​bottom-tabs/​react-navigation@​1.1.0941007396100
Added@​aptos-labs/​siwa@​0.4.0771007387100
Added@​babel/​preset-typescript@​7.27.11001007394100
Addedesbuild@​0.27.2911007390100
Updated@​babel/​plugin-transform-optional-chaining@​7.27.1 ⏵ 7.25.9100 +110074 +194100
Added@​formatjs/​intl-pluralrules@​4.3.31001007492100
Addedexpo-keep-awake@​14.1.4741008299100
Added@​formatjs/​intl-locale@​2.4.471001007592100
Added@​formatjs/​intl-getcanonicallocales@​1.9.21001007692100
Added@​babel/​preset-env@​7.28.6971007795100
Added@​glif/​filecoin-rpc-client@​3.0.27710010081100
Added@​benfen/​bfc.js@​0.2.7821007789100
Added@​alephium/​web3@​1.5.292100779670
Added@​babel/​core@​7.27.1971008095100
Added@​glif/​filecoin-message@​2.0.44801008184100
Added@​electron/​remote@​2.1.110010010081100
Added@​cityofzion/​neon-core@​5.7.0861008186100
Added@​ckb-lumos/​ckb-indexer@​0.23.08110010083100
Added@​emurgo/​cardano-message-signing-nodejs@​1.0.1911008285100
Added@​cowprotocol/​contracts@​1.7.0881001008470
Added@​ckb-lumos/​common-scripts@​0.23.08310010083100
Added@​electron/​notarize@​2.3.210010010083100
Added@​aivenio/​tsc-output-parser@​2.1.19910010084100
Added@​glif/​filecoin-address@​2.0.43841008584100
See 9 more rows in the dashboard

View full report

@socket-security

Copy link
Copy Markdown

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Warn Critical
Critical CVE: Elliptic's private key extraction in ECDSA upon signing a malformed input (e.g. a string)

CVE: GHSA-vjh7-7g9h-fjfh Elliptic's private key extraction in ECDSA upon signing a malformed input (e.g. a string) (CRITICAL)

Affected versions: < 6.6.1

Patched version: 6.6.1

From: ?npm/elliptic@6.5.4

ℹ Read more on: This package | This alert | What is a critical CVE?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/elliptic@6.5.4. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn High
Obfuscated code: npm @emurgo/cardano-message-signing-asmjs is 90.0% likely obfuscated

Confidence: 0.90

Location: Package overview

From: ?npm/@emurgo/cardano-message-signing-asmjs@1.0.1

ℹ Read more on: This package | This alert | What is obfuscated code?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@emurgo/cardano-message-signing-asmjs@1.0.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@originalix originalix changed the title feat(btc): expose coinselect sortingStrategy for deterministic output ordering feat(btc): expose coinselect sortingStrategy for deterministic output ordering, OK-50996 Mar 5, 2026
@originalix
originalix disabled auto-merge March 5, 2026 07:21
@originalix
originalix merged commit c9553a5 into x Mar 5, 2026
11 checks passed
@originalix
originalix deleted the feat/btc-coinselect-sorting-strategy branch March 5, 2026 07:21
PatrickChoo pushed a commit to PatrickChoo/app-monorepo that referenced this pull request Mar 10, 2026
… ordering, OK-50996 (OneKeyHQ#10493)

* docs: add coinselect sorting strategy design doc

Document the plan to expose sortingStrategy parameter in
@onekeyfe/coinselect witness.js for deterministic output ordering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* docs: add coinselect sortingStrategy implementation plan

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(btc): pass sortingStrategy to coinSelectWithWitness for swap transactions

Expose sortingStrategy parameter through coinSelectWithWitness to control
BTC transaction output ordering. When opReturn is present (protocol
transactions like THORSwap), use 'none' strategy to guarantee the payment
address appears as the first output. Regular transfers continue using the
default 'random' strategy for privacy.

* chore: upgrade sdk

* chore: delete plan docs

---------

Co-authored-by: Claude Opus 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.

4 participants