Skip to content

backport: Merge bitcoin#26053, 28230, 28500 #7256

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

Merged
PastaPastaPasta merged 3 commits into from
Jun 5, 2026
Merged

backport: Merge bitcoin#26053, 28230, 28500 #7256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcf55ba
Merge bitcoin/bitcoin#26053: rpc: bugfix, 'add_inputs' default value …
achow101 Sep 14, 2022
3fe49c9
Merge bitcoin/bitcoin#28230: rpc: Add MaybeArg() and Arg() default he…
fanquake Aug 24, 2023
e6df404
Merge bitcoin/bitcoin#28500: Prevent default/invalid CKey objects fro…
fanquake Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/wallet/rpc/spend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ RPCHelpMan send()
{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
Cat<std::vector<RPCArg>>(
{
{"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{false}, "If inputs are specified, automatically include more if they are not enough."},
{"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"},"Automatically include coins from the wallet to cover the target amount.\n"},
{"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n"
"Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n"
"If that happens, you will need to fund the transaction with different inputs and republish it."},
Expand Down Expand Up @@ -1377,7 +1377,7 @@ RPCHelpMan walletcreatefundedpsbt()
{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
Cat<std::vector<RPCArg>>(
{
{"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{false}, "If inputs are specified, automatically include more if they are not enough."},
{"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"}, "Automatically include coins from the wallet to cover the target amount.\n"},
{"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n"
"Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n"
"If that happens, you will need to fund the transaction with different inputs and republish it."},
Expand Down
115 changes: 115 additions & 0 deletions test/functional/wallet_fundrawtransaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def run_test(self):
self.generate(self.nodes[2], 1)
self.generate(self.nodes[0], 121)

self.test_add_inputs_default_value()
self.test_change_position()
self.test_simple()
self.test_simple_two_coins()
Expand Down Expand Up @@ -1021,6 +1022,120 @@ def test_external_inputs(self):

self.nodes[2].unloadwallet("extfund")

def test_add_inputs_default_value(self):
self.log.info("Test 'add_inputs' default value")

# Create and fund the wallet with 5 BTC
self.nodes[2].createwallet("test_preset_inputs")
wallet = self.nodes[2].get_wallet_rpc("test_preset_inputs")
addr1 = wallet.getnewaddress()
self.nodes[0].sendtoaddress(addr1, 5)
self.generate(self.nodes[0], 1)

# Covered cases:
# 1. Default add_inputs value with no preset inputs (add_inputs=true):
# Expect: automatically add coins from the wallet to the tx.
# 2. Default add_inputs value with preset inputs (add_inputs=false):
# Expect: disallow automatic coin selection.
# 3. Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount).
# Expect: include inputs from the wallet.
# 4. Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount).
# Expect: only preset inputs are used.
# 5. Explicit add_inputs=true, no preset inputs (same as (1) but with an explicit set):
# Expect: include inputs from the wallet.

# Case (1), 'send' command
# 'add_inputs' value is true unless "inputs" are specified, in such case, add_inputs=false.
# So, the wallet will automatically select coins and create the transaction if only the outputs are provided.
tx = wallet.send(outputs=[{addr1: 3}])
assert tx["complete"]

# Case (2), 'send' command
# Select an input manually, which doesn't cover the entire output amount and
# verify that the dynamically set 'add_inputs=false' value works.

# Fund wallet with 2 outputs, 5 BTC each.
addr2 = wallet.getnewaddress()
source_tx = self.nodes[0].send(outputs=[{addr1: 5}, {addr2: 5}], options={"change_position": 0})
self.generate(self.nodes[0], 1)

# Select only one input.
options = {
"inputs": [
{
"txid": source_tx["txid"],
"vout": 1 # change position was hardcoded to index 0
}
]
}
assert_raises_rpc_error(-4, "Insufficient funds", wallet.send, outputs=[{addr1: 8}], options=options)

# Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount)
options["add_inputs"] = True
options["add_to_wallet"] = False
tx = wallet.send(outputs=[{addr1: 8}], options=options)
assert tx["complete"]

# Case (4), Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount)
options["inputs"].append({
"txid": source_tx["txid"],
"vout": 2 # change position was hardcoded to index 0
})
tx = wallet.send(outputs=[{addr1: 8}], options=options)
assert tx["complete"]
# Check that only the preset inputs were added to the tx
decoded_psbt_inputs = self.nodes[0].decodepsbt(tx["psbt"])['tx']['vin']
assert_equal(len(decoded_psbt_inputs), 2)
for input in decoded_psbt_inputs:
assert_equal(input["txid"], source_tx["txid"])
Comment on lines +1089 to +1090

@coderabbitai coderabbitai Bot Apr 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify builtin-shadowing occurrences in this test file
rg -nP '\bfor\s+input\s+in\b' test/functional/wallet_fundrawtransaction.py

Repository: dashpay/dash

Length of output: 1769

🏁 Script executed:

#!/bin/bash
# Search for 'for input in' patterns in the wallet_fundrawtransaction.py test file
cd test/functional
if [ -f wallet_fundrawtransaction.py ]; then
    echo "=== Searching for 'for input in' patterns ==="
    rg -n 'for\s+input\s+in' wallet_fundrawtransaction.py
    echo ""
    echo "=== Context around first match (lines 1085-1095) ==="
    sed -n '1085,1095p' wallet_fundrawtransaction.py
    echo ""
    echo "=== Context around second match (lines 1124-1134) ==="
    sed -n '1124,1134p' wallet_fundrawtransaction.py
else
    echo "File not found"
fi

Repository: dashpay/dash

Length of output: 1445

Fix Ruff A001: avoid shadowing builtin input.

Lines 1089 and 1128 use input as a loop variable, which shadows Python's builtin input() function and triggers Ruff A001 linting violations.

Proposed rename 
-        for input in decoded_psbt_inputs:
-            assert_equal(input["txid"], source_tx["txid"])
+        for vin in decoded_psbt_inputs:
+            assert_equal(vin["txid"], source_tx["txid"])

This applies to both occurrences at lines 1089 and 1128.

📝 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
for input in decoded_psbt_inputs:
assert_equal(input["txid"], source_tx["txid"])
for vin in decoded_psbt_inputs:
assert_equal(vin["txid"], source_tx["txid"])
🧰 Tools
🪛 Ruff (0.15.9)

[error] 1089-1089: Variable input is shadowing a Python builtin

(A001)

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

In `@test/functional/wallet_fundrawtransaction.py` around lines 1089 - 1090, The
loop variable named input in test/functional/wallet_fundrawtransaction.py
shadows the built-in input() (Ruff A001); rename the loop variable (both
occurrences used over decoded_psbt_inputs) to a non-builtin name like psbt_input
or decoded_input and update any uses inside the loop (e.g., input["txid"])
accordingly so the tests reference the new variable name everywhere within each
loop (check both the loop at the decoded_psbt_inputs iteration and the later
loop around line 1128).


# Case (5), assert that inputs are added to the tx by explicitly setting add_inputs=true
options = {"add_inputs": True, "add_to_wallet": True}
tx = wallet.send(outputs=[{addr1: 8}], options=options)
assert tx["complete"]

################################################

# Case (1), 'walletcreatefundedpsbt' command
# Default add_inputs value with no preset inputs (add_inputs=true)
inputs = []
outputs = {self.nodes[1].getnewaddress(): 8}
assert "psbt" in wallet.walletcreatefundedpsbt(inputs=inputs, outputs=outputs)

# Case (2), 'walletcreatefundedpsbt' command
# Default add_inputs value with preset inputs (add_inputs=false).
inputs = [{
"txid": source_tx["txid"],
"vout": 1 # change position was hardcoded to index 0
}]
outputs = {self.nodes[1].getnewaddress(): 8}
assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, inputs=inputs, outputs=outputs)

# Case (3), Explicit add_inputs=true and preset inputs (with preset inputs not-covering the target amount)
options["add_inputs"] = True
options["add_to_wallet"] = False
assert "psbt" in wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, options=options)

# Case (4), Explicit add_inputs=true and preset inputs (with preset inputs covering the target amount)
inputs.append({
"txid": source_tx["txid"],
"vout": 2 # change position was hardcoded to index 0
})
psbt_tx = wallet.walletcreatefundedpsbt(outputs=[{addr1: 8}], inputs=inputs, options=options)
# Check that only the preset inputs were added to the tx
decoded_psbt_inputs = self.nodes[0].decodepsbt(psbt_tx["psbt"])['tx']['vin']
assert_equal(len(decoded_psbt_inputs), 2)
for input in decoded_psbt_inputs:
assert_equal(input["txid"], source_tx["txid"])

# Case (5), 'walletcreatefundedpsbt' command
# Explicit add_inputs=true, no preset inputs
options = {
"add_inputs": True
}
assert "psbt" in wallet.walletcreatefundedpsbt(inputs=[], outputs=outputs, options=options)

self.nodes[2].unloadwallet("test_preset_inputs")

@knst knst Jun 2, 2026

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.

26053: nit: missing \n before test_include_unsafe

def test_include_unsafe(self):
self.log.info("Test fundrawtxn with unsafe inputs")

Expand Down