Skip to content

partial bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-ups - #1208

Merged
PastaPastaPasta merged 0 commit into
backport/auto-20251204from
backport-0.24-batch-463-pr-25734
Dec 4, 2025
Merged

PastaPastaPasta merged 0 commit into
backport/auto-20251204from
backport-0.24-batch-463-pr-25734

Conversation

@DashCoreAutoGuix

Copy link
Copy Markdown
Owner

Summary

Backport of Bitcoin PR bitcoin#25734 to Dash.

This PR refactors CoinsResult to use a std::map<OutputType, std::vector<COutput>> storage instead of individual named vectors, and adds helper methods for common operations.

Dash-specific adaptations:

  • Removed SegWit-related OutputTypes (P2SH_SEGWIT, BECH32, BECH32M) - Dash only supports LEGACY
  • Removed sendall RPC command (SegWit-specific feature not applicable to Dash)
  • Adapted GetOutputType() to only handle LEGACY and UNKNOWN types
  • Changed OUTPUT_TYPES from extern const array to static constexpr
  • Added CoinsResult helper methods: All(), Size(), Clear(), Erase(), Shuffle(), Add()
  • Updated all callers across the codebase to use new capitalized method names (all() → All(), size() → Size(), etc.)
  • Additional Dash-specific files updated: src/coinjoin/client.cpp, src/wallet/coinjoin.cpp, src/rpc/evo.cpp, src/rpc/masternode.cpp

Change metrics:

  • Bitcoin: 343 lines changed (13 files)
  • Dash: 302 lines changed (13 files)
  • Ratio: 88% (within acceptable 80-150% range)

Bitcoin PR

bitcoin#25734

Test plan

  • Build succeeds
  • Coin selector unit tests pass
  • Available coins unit tests pass
  • Wallet unit tests pass

@coderabbitai

coderabbitai Bot commented Dec 1, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@DashCoreAutoGuix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 35c7990 and 03a5587.

📒 Files selected for processing (13)
  • src/bench/coin_selection.cpp (1 hunks)
  • src/coinjoin/client.cpp (1 hunks)
  • src/outputtype.cpp (1 hunks)
  • src/outputtype.h (1 hunks)
  • src/rpc/evo.cpp (1 hunks)
  • src/rpc/masternode.cpp (1 hunks)
  • src/wallet/coinjoin.cpp (3 hunks)
  • src/wallet/rpc/coins.cpp (1 hunks)
  • src/wallet/spend.cpp (8 hunks)
  • src/wallet/spend.h (1 hunks)
  • src/wallet/test/availablecoins_tests.cpp (2 hunks)
  • src/wallet/test/coinselector_tests.cpp (18 hunks)
  • src/wallet/test/wallet_tests.cpp (4 hunks)
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.24-batch-463-pr-25734

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DashCoreAutoGuix

Copy link
Copy Markdown
Owner Author

Verification Complete ✅

Bitcoin Commit: 64f7a19 (PR bitcoin#25734)
CI Status: All 53 checks passing ✅

Summary

This PR successfully backports Bitcoin PR bitcoin#25734 (wallet refactor follow-ups to bitcoin#24584), which adds the UNKNOWN OutputType and refactors CoinsResult methods for better code organization.

Changes Verified

Core Backport:

  • ✅ Added UNKNOWN to OUTPUT_TYPES array and parsing
  • ✅ Refactored CoinsResult with new methods: .All(), .Size(), .Clear(), .Erase(), .Shuffle(), .Add()
  • ✅ Simplified OutputType handling for Dash's non-SegWit architecture
  • ✅ All tests updated appropriately

Dash-Specific Adaptations:

  • ✅ Updated CoinJoin code to use new API (src/coinjoin/client.cpp, src/wallet/coinjoin.cpp)
  • ✅ Updated masternode RPCs to use new API (src/rpc/masternode.cpp)
  • ✅ Updated Evolution RPCs to use new API (src/rpc/evo.cpp)

Intentionally Excluded:

  • Bitcoin's descriptor wallet changes in scriptpubkeyman.cpp and wallet.cpp (Dash has different descriptor architecture)
  • Bitcoin's wallet_avoid_mixing_output_types.py test (requires full descriptor wallet support)
  • Changes to sendall RPC in spend.cpp (function doesn't exist in current Dash)

Validation

  • Size ratio: 88% (within acceptable range)
  • File adaptations properly account for Dash's architectural differences
  • All CI checks passing
  • No conflicting changes

This is a clean, well-adapted backport ready for merge.

@DashCoreAutoGuix DashCoreAutoGuix added the verified Backport verification passed - ready for merge label Dec 2, 2025

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

light-utACK

@knst knst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mark as partial, otherwise it looks good for me.

Missing changes (beside taproot) are:

  • src/wallet/scriptpubkeyman.cpp
  • src/wallet/wallet.cpp

@PastaPastaPasta
PastaPastaPasta changed the base branch from backport-0.24-batch-463 to backport/auto-20251204 December 4, 2025 16:41
@PastaPastaPasta PastaPastaPasta changed the title Merge bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-ups partial bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-ups Dec 4, 2025
@PastaPastaPasta
PastaPastaPasta merged this pull request into backport/auto-20251204 Dec 4, 2025
53 checks passed
@PastaPastaPasta
PastaPastaPasta deleted the backport-0.24-batch-463-pr-25734 branch December 4, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

verified Backport verification passed - ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants