Skip to content

backport: Merge bitcoin#29227, (partial) 27389, 27832, 30215#7202

Open
vijaydasmp wants to merge 4 commits intodashpay:developfrom
vijaydasmp:March_2026_8
Open

backport: Merge bitcoin#29227, (partial) 27389, 27832, 30215#7202
vijaydasmp wants to merge 4 commits intodashpay:developfrom
vijaydasmp:March_2026_8

Conversation

@vijaydasmp
Copy link

bitcoin backporting

@github-actions
Copy link

github-actions bot commented Mar 6, 2026

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@vijaydasmp vijaydasmp force-pushed the March_2026_8 branch 2 times, most recently from 3d7e8ad to 00f1792 Compare March 6, 2026 07:18
@vijaydasmp vijaydasmp marked this pull request as ready for review March 6, 2026 09:50
@coderabbitai
Copy link

coderabbitai bot commented Mar 6, 2026

Walkthrough

This pull request updates JSON-RPC examples and generated help to use Content-Type: application/json instead of text/plain (docs, rpc util, and RPC help tests). The -datacarriersize CLI help text was rewritten for clarity only. Mempool dump loading in validation.cpp was refactored to iterate with an explicit tried counter and to emit periodic progress logs at 10% granularity. Functional tests were adjusted: binary /getutxos request/response handling moved from struct/BytesIO to explicit byte operations, and the mempool datacarrier functional test was expanded to four nodes, added scenarios for null/zero/one-byte OP_RETURN payloads, and relaxed a test method's parameter typing.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description 'bitcoin backporting' is vague and generic, providing no meaningful information about what is being backported or why. Expand the description to explain which upstream changes are being merged and their purpose, e.g., 'Backport of bitcoin#29227, #27389, #27832, #30215 for improved mempool logging, test refactoring, and RPC documentation updates.'
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately identifies this as a backport of four Bitcoin PRs and lists them by number, clearly conveying the main change.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
src/rpc/util.cpp (1)

222-234: Port number inconsistency between HelpExampleRpc and HelpExampleRpcNamed.

HelpExampleRpc on line 223 uses port 9998 (Dash mainnet RPC), while HelpExampleRpcNamed on line 234 uses port 8332 (Bitcoin mainnet RPC). This inconsistency appears to predate this PR but is worth noting since this code is being modified.

The content-type change from text/plain to application/json is correct for JSON-RPC requests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/rpc/util.cpp` around lines 222 - 234, The two example helpers use
different hard-coded ports—HelpExampleRpc uses 9998 while HelpExampleRpcNamed
uses 8332—so make them consistent: either replace the 9998 in HelpExampleRpc
with 8332 to match HelpExampleRpcNamed, or (preferred) use the shared RPC port
constant/variable used elsewhere (instead of a literal) in both HelpExampleRpc
and HelpExampleRpcNamed so both examples always show the correct configured RPC
port.
test/functional/mempool_datacarrier.py (2)

33-36: Consider using list unpacking for cleaner syntax.

The type hint was removed to accommodate None, and the concatenation works correctly. However, using unpacking is more idiomatic Python.

Proposed changes
-    def test_null_data_transaction(self, node: TestNode, data, success: bool) -> None:
+    def test_null_data_transaction(self, node: TestNode, data: bytes | None, success: bool) -> None:
         tx = self.wallet.create_self_transfer(fee_rate=0)["tx"]
         data = [] if data is None else [data]
-        tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN] + data)))
+        tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, *data])))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/functional/mempool_datacarrier.py` around lines 33 - 36, The
test_null_data_transaction function currently builds the OP_RETURN script by
concatenating a list ([OP_RETURN] + data) after normalizing data to a list;
replace that concatenation with Python list unpacking to be more idiomatic: keep
the data normalization (data = [] if data is None else [data]) and construct the
CScript using CScript([OP_RETURN, *data]) when creating the CTxOut (refer to
test_null_data_transaction, tx.vout.append(CTxOut(...)), CScript and data
variables).

30-30: Remove extraneous f-string prefix.

This string has no placeholders, so the f prefix is unnecessary.

Proposed fix
-            ["-datacarrier=1", f"-datacarriersize=2"],
+            ["-datacarrier=1", "-datacarriersize=2"],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/functional/mempool_datacarrier.py` at line 30, The second list element
uses an unnecessary f-string (f"-datacarriersize=2") even though there are no
placeholders; replace f"-datacarriersize=2" with a plain string
"-datacarriersize=2" in the list (e.g., change ["-datacarrier=1",
f"-datacarriersize=2"] to ["-datacarrier=1", "-datacarriersize=2"]).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/rpc/util.cpp`:
- Around line 222-234: The two example helpers use different hard-coded
ports—HelpExampleRpc uses 9998 while HelpExampleRpcNamed uses 8332—so make them
consistent: either replace the 9998 in HelpExampleRpc with 8332 to match
HelpExampleRpcNamed, or (preferred) use the shared RPC port constant/variable
used elsewhere (instead of a literal) in both HelpExampleRpc and
HelpExampleRpcNamed so both examples always show the correct configured RPC
port.

In `@test/functional/mempool_datacarrier.py`:
- Around line 33-36: The test_null_data_transaction function currently builds
the OP_RETURN script by concatenating a list ([OP_RETURN] + data) after
normalizing data to a list; replace that concatenation with Python list
unpacking to be more idiomatic: keep the data normalization (data = [] if data
is None else [data]) and construct the CScript using CScript([OP_RETURN, *data])
when creating the CTxOut (refer to test_null_data_transaction,
tx.vout.append(CTxOut(...)), CScript and data variables).
- Line 30: The second list element uses an unnecessary f-string
(f"-datacarriersize=2") even though there are no placeholders; replace
f"-datacarriersize=2" with a plain string "-datacarriersize=2" in the list
(e.g., change ["-datacarrier=1", f"-datacarriersize=2"] to ["-datacarrier=1",
"-datacarriersize=2"]).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0120a33b-a7ca-4afc-a648-f0a6b9fa389c

📥 Commits

Reviewing files that changed from the base of the PR and between af926ae and 00f1792.

📒 Files selected for processing (7)
  • doc/JSON-RPC-interface.md
  • src/init.cpp
  • src/rpc/util.cpp
  • src/test/rpc_tests.cpp
  • src/validation.cpp
  • test/functional/interface_rest.py
  • test/functional/mempool_datacarrier.py

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 00f1792

for txid, n in [spending, spent]:
bin_request += bytes.fromhex(txid)
bin_request += pack("i", n)
bin_request += n.to_bytes(4, 'little')
Copy link
Collaborator

Choose a reason for hiding this comment

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

27389: should be marked as partial

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

lgtm overal; 27389 should be marked as partial

eb78ea4 [log] mempool loading (glozow)

Pull request description:

  Motivated by bitcoin#29193. Currently, we only log something (non-debug) when we fail to load the file and at the end of importing all the transactions. That means it's hard to tell what's happening if it's taking a long time to load.

  This PR adds a maximum of 10 new unconditional log lines:
  - When we start to load transactions.
  - Our progress percentage when it advances by at least 10% from the last time we logged. Percentage is based on the number of transactions.

  If there are lots of transactions in the mempool, the logs will look like this:
  ```
  2024-01-11T11:36:30.410726Z Loading 401 mempool transactions from disk...
  2024-01-11T11:36:30.423374Z Progress loading mempool transactions from disk: 10% (tried 41, 360 remaining)
  2024-01-11T11:36:30.435539Z Progress loading mempool transactions from disk: 20% (tried 81, 320 remaining)
  2024-01-11T11:36:30.447874Z Progress loading mempool transactions from disk: 30% (tried 121, 280 remaining)
  2024-01-11T11:36:30.460474Z Progress loading mempool transactions from disk: 40% (tried 161, 240 remaining)
  2024-01-11T11:36:30.473731Z Progress loading mempool transactions from disk: 50% (tried 201, 200 remaining)
  2024-01-11T11:36:30.487806Z Progress loading mempool transactions from disk: 60% (tried 241, 160 remaining)
  2024-01-11T11:36:30.501739Z Progress loading mempool transactions from disk: 70% (tried 281, 120 remaining)
  2024-01-11T11:36:30.516334Z Progress loading mempool transactions from disk: 80% (tried 321, 80 remaining)
  2024-01-11T11:36:30.531309Z Progress loading mempool transactions from disk: 90% (tried 361, 40 remaining)
  2024-01-11T11:36:30.549019Z  Imported mempool transactions from disk: 401 succeeded, 0 failed, 0 expired, 0 already there, 400 waiting for initial broadcast
  ```
  If there are 0 or 1 transactions, progress logs aren't printed.

ACKs for top commit:
  kevkevinpal:
    Concept ACK [eb78ea4](bitcoin@eb78ea4)
  ismaelsadeeq:
    ACK eb78ea4
  dergoegge:
    Code review ACK eb78ea4
  theStack:
    re-ACK eb78ea4
  mzumsande:
    tested ACK eb78ea4

Tree-SHA512: ae4420986dc7bd5cb675a7ebc76b24c8ee60007f0296ed37e272f1c3415764d44963bea84c51948da319a65661dca8a95eac2a59bf7e745519b6fcafa09812cf
…ytesIO` uses

f842ed9 test: refactor: replace unnecessary `BytesIO` uses (Sebastian Falbesoner)

Pull request description:

  Rather than needing to create intermediate stream variables, we can use helper functions like `tx_from_hex` instead or access the result directly, leading both to increased readability and less code.

ACKs for top commit:
  stickies-v:
    ACK f842ed9
  brunoerg:
    crACK f842ed9
  aureleoules:
    ACK f842ed9 - It seems that these are the only instances that can be changed and it simplifies test code.

Tree-SHA512: 7f4fd7a26720d1988bf27f66c817ff6cd7060350a3be62d28e7848c768fd43578719ca475193d4057ccf4f2458af18564fd513fe3a1d458a11c799927c12bd65
…ize=2 tests

faafc35 doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not the data push (MarcoFalke)
55550e7 test: Add -datacarriersize=2 tests (MarcoFalke)

Pull request description:

  Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example,

  * `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed
  * `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed
  * `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed

ACKs for top commit:
  ajtowns:
    ACK faafc35
  instagibbs:
    ACK bitcoin@faafc35

Tree-SHA512: f01ace02798f596ac2a02461e9f2a6ef91b3b37c976ea0b3bc860e2d3efb0ace0fd8b779dd18249cee7f84ebbe5fd21d8506afd3a15edadc00b843ff3b4aacc7
…n/json

3c08e11 doc: JSON-RPC request Content-Type is application/json (Luke Dashjr)

Pull request description:

  Specify json content type in RPC examples.

  Picks up bitcoin#29946. Which needed rebasing and the commit message fixing,

ACKs for top commit:
  laanwj:
    ACK 3c08e11
  tdb3:
    ACK for 3c08e11

Tree-SHA512: 770bbbc0fb324cb63628980b13583cabf02e75079851850170587fb6eca41a70b01dcedaf1926bb6488eb9816a3cc6616fe8cee8c4b7e09aa39b7df5834ca0ec
@vijaydasmp vijaydasmp changed the title backport: Merge bitcoin#29227, 27389, 27832, 30215 backport: Merge bitcoin#29227, (partial) 27389, 27832, 30215 Mar 25, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/functional/mempool_datacarrier.py`:
- Line 30: Remove the unnecessary f-string prefix from the datacarriersize
argument: change the list element f"-datacarriersize=2" to a plain string
"-datacarriersize=2" in the array that currently contains ["-datacarrier=1",
f"-datacarriersize=2"] so the code no longer triggers the F541 unused f-string
warning.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4f25b02a-da23-4af2-bfab-5ec9225cb8f7

📥 Commits

Reviewing files that changed from the base of the PR and between 00f1792 and 73e7bf5.

📒 Files selected for processing (7)
  • doc/JSON-RPC-interface.md
  • src/init.cpp
  • src/rpc/util.cpp
  • src/test/rpc_tests.cpp
  • src/validation.cpp
  • test/functional/interface_rest.py
  • test/functional/mempool_datacarrier.py
✅ Files skipped from review due to trivial changes (4)
  • doc/JSON-RPC-interface.md
  • src/rpc/util.cpp
  • src/test/rpc_tests.cpp
  • src/init.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/validation.cpp

["-datacarrier=0"],
["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"]
["-datacarrier=1", f"-datacarriersize={MAX_OP_RETURN_RELAY - 1}"],
["-datacarrier=1", f"-datacarriersize=2"],
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Expect one hit before the fix and no output after the fix.
python - <<'PY'
import ast
from pathlib import Path

path = Path("test/functional/mempool_datacarrier.py")
tree = ast.parse(path.read_text())
for node in ast.walk(tree):
    if isinstance(node, ast.JoinedStr) and not any(isinstance(v, ast.FormattedValue) for v in node.values):
        print(f"{path}:{node.lineno}: placeholder-free f-string")
PY

Repository: dashpay/dash

Length of output: 124


Remove the unused f prefix on line 30.

This f-string has no placeholders and triggers Ruff/Flake8 F541.

Suggested fix
-            ["-datacarrier=1", f"-datacarriersize=2"],
+            ["-datacarrier=1", "-datacarriersize=2"],
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
["-datacarrier=1", f"-datacarriersize=2"],
["-datacarrier=1", "-datacarriersize=2"],
🧰 Tools
🪛 Flake8 (7.3.0)

[error] 30-30: f-string is missing placeholders

(F541)

🪛 Ruff (0.15.6)

[error] 30-30: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/functional/mempool_datacarrier.py` at line 30, Remove the unnecessary
f-string prefix from the datacarriersize argument: change the list element
f"-datacarriersize=2" to a plain string "-datacarriersize=2" in the array that
currently contains ["-datacarrier=1", f"-datacarriersize=2"] so the code no
longer triggers the F541 unused f-string warning.

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