Skip to content

Merge bitcoin/bitcoin#25379: test: use MiniWallet to simplify mempool_package_limits.py tests#1202

Open
DashCoreAutoGuix wants to merge 1 commit intobackport-0.24-batch-463from
backport-0.24-batch-463-pr-25379
Open

Merge bitcoin/bitcoin#25379: test: use MiniWallet to simplify mempool_package_limits.py tests#1202
DashCoreAutoGuix wants to merge 1 commit intobackport-0.24-batch-463from
backport-0.24-batch-463-pr-25379

Conversation

@DashCoreAutoGuix
Copy link
Copy Markdown
Owner

Backports bitcoin#25379

Original commit: 9155f9b

Summary

  • Simplifies mempool_package_limits.py functional tests with MiniWallet usage
  • Adds target_weight parameter to create_self_transfer and create_self_transfer_multi methods
  • Removes standalone bulk_transaction function in favor of _bulk_tx method in MiniWallet
  • Adds send_self_transfer_chain method for creating chains of transactions
  • Removes unused helper functions no longer needed with MiniWallet

Changes from Bitcoin

  • Removed WITNESS_SCALE_FACTOR references (Dash doesn't have SegWit)
  • Kept helper functions make_chain, create_child_with_parents, create_raw_chain as they're still used by other Dash tests
  • Adapted target_weight calculation without witness scale factor

Scope

  • Bitcoin: 367 lines changed
  • Dash: 373 lines changed
  • Ratio: 101% (within acceptable range)

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 1, 2025

Warning

Rate limit exceeded

@DashCoreAutoGuix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 56 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 2241b09.

📒 Files selected for processing (2)
  • test/functional/mempool_package_limits.py (9 hunks)
  • test/functional/test_framework/wallet.py (6 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-25379

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

Dependency Issue

This PR backports Bitcoin bitcoin#25379, which introduces parameter to MiniWallet's methods. However, this implementation has a known issue: when is used without explicitly specifying a parameter, the transaction fee is calculated for the base transaction size (before padding), resulting in an insufficient fee rate after padding.

Test Failure

The test fails with:

JSONRPCException: min relay fee not met, 25500 < 30332 (-26)

This occurs because:

  1. A transaction with target_weight=30000 (30KB) is created using default fee_rate=0.003
  2. The fee is calculated for the base transaction (~168 bytes), resulting in ~504 satoshis
  3. The transaction is then padded to 30KB
  4. The effective fee rate drops to ~0.017 sats/byte, below the minimum relay fee

Required Dependency

Bitcoin PR bitcoin#30162 (e6e4c18a9b) fixes this issue by making MiniWallet properly calculate fees when both target_weight and fee_rate are specified together. That PR was merged on 2024-06-11, nearly 2 years after bitcoin#25379 (merged 2022-08-03).

This PR requires Bitcoin bitcoin#30162 to be backported first.

Timeline

@DashCoreAutoGuix
Copy link
Copy Markdown
Owner Author

Dependency Issue

This PR backports Bitcoin bitcoin#25379, which introduces the target_weight parameter to MiniWallet's create_self_transfer methods. However, this implementation has a known issue: when target_weight is used without explicitly specifying a fee parameter, the transaction fee is calculated for the base transaction size (before padding), resulting in an insufficient fee rate after padding.

Test Failure

The test mempool_package_limits.py test_anc_size_limits() fails with:

JSONRPCException: min relay fee not met, 25500 < 30332 (-26)

This occurs because:

  1. A transaction with target_weight=30000 (30KB) is created using default fee_rate=0.003
  2. The fee is calculated for the base transaction (~168 bytes), resulting in ~504 satoshis
  3. The transaction is then padded to 30KB
  4. The effective fee rate drops to ~0.017 sats/byte, below the minimum relay fee

Required Dependency

Bitcoin PR bitcoin#30162 (commit e6e4c18) fixes this issue by making MiniWallet properly calculate fees when both target_weight and fee_rate are specified together. That PR was merged on 2024-06-11, nearly 2 years after bitcoin#25379 (merged 2022-08-03).

This PR requires Bitcoin bitcoin#30162 to be backported first.

Timeline

@DashCoreAutoGuix DashCoreAutoGuix added needs-dependency Requires a prerequisite Bitcoin PR to be backported first blocked PR is blocked and cannot proceed labels Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked PR is blocked and cannot proceed needs-dependency Requires a prerequisite Bitcoin PR to be backported first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant