Skip to content

Merge bitcoin/bitcoin#26733: test: Add test for sendmany rpc that uses subtractfeefrom parameter #1137

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

Open
DashCoreAutoGuix wants to merge 3 commits into
base: backport-0.25-batch-415
Choose a base branch
from
Open

Merge bitcoin/bitcoin#26733: test: Add test for sendmany rpc that uses subtractfeefrom parameter #1137

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac6847e
Merge bitcoin/bitcoin#26733: test: Add test for `sendmany` rpc that u…
achow101 May 1, 2023
c04fd61
validation: Fix decimal precision issue in sendmany test
claude Dec 2, 2025
8e1fd4c
validation: Fix sendmany test to handle uneven fee splitting
claude Dec 2, 2025
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
15 changes: 15 additions & 0 deletions test/functional/wallet_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
assert_raises_rpc_error,
count_bytes,
find_vout_for_address,
satoshi_round,
)
Comment on lines +19 to 20

@coderabbitai coderabbitai Bot Dec 2, 2025

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

Scope creep: satoshi_round usage diverges from Bitcoin commit 5394522

In Bitcoin’s wallet_basic.py for this commit, the line is:

  • expected_bal = Decimal('5') + (tx['fee'] / 2)

The Dash backport instead wraps this in satoshi_round(...) and adds a new import, which is a behavioral change not present in the original commit and thus scope creep for this backport.

Unless there is a Dash-specific consensus/amount representation issue that absolutely requires it, this should be reverted to preserve a faithful backport and keep the test logic identical to Bitcoin’s.

Suggested fix (and then drop the now-unused import of satoshi_round):

-from test_framework.util import (
-    assert_array_result,
-    assert_equal,
-    assert_fee_amount,
-    assert_greater_than,
-    assert_raises_rpc_error,
-    count_bytes,
-    find_vout_for_address,
-    satoshi_round,
-)
+from test_framework.util import (
+    assert_array_result,
+    assert_equal,
+    assert_fee_amount,
+    assert_greater_than,
+    assert_raises_rpc_error,
+    count_bytes,
+    find_vout_for_address,
+)
@@
-        expected_bal = satoshi_round(Decimal('5') + (tx['fee'] / 2))
+        expected_bal = Decimal('5') + (tx['fee'] / 2)

If you do need rounding to work around a Dash peculiarity, that should be documented and preferably done in a follow-up Dash-specific commit, not inside this pure Bitcoin backport.

Also applies to: 290-292

🤖 Prompt for AI Agents 
In test/functional/wallet_basic.py around lines 19-20 (and also check 290-292),
the test was changed to wrap the expected balance calculation in satoshi_round
which diverges from the original Bitcoin commit; replace the satoshi_round(...)
usage with the original expression expected_bal = Decimal('5') + (tx['fee'] / 2)
and remove the now-unused import of satoshi_round (and any other occurrences
added at lines 290-292); if Dash-specific rounding is required, implement and
document it in a separate Dash-only commit instead of this Bitcoin backport.

from test_framework.wallet_util import test_address

Expand Down Expand Up @@ -276,6 +277,20 @@ def run_test(self):
assert_equal(self.nodes[2].getbalance(), node_2_bal)
node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('100'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex']))

# Sendmany 5 DASH to two addresses with subtracting fee from both addresses
a0 = self.nodes[0].getnewaddress()
a1 = self.nodes[0].getnewaddress()
txid = self.nodes[2].sendmany(dummy='', amounts={a0: 5, a1: 5}, subtractfeefrom=[a0, a1])
self.generate(self.nodes[2], 1, sync_fun=lambda: self.sync_all(self.nodes[0:3]))
node_2_bal -= Decimal('10')
assert_equal(self.nodes[2].getbalance(), node_2_bal)
tx = self.nodes[2].gettransaction(txid)
node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(tx['hex']))
assert_equal(self.nodes[0].getbalance(), node_0_bal)
expected_bal = satoshi_round(Decimal('5') + (tx['fee'] / 2))
assert_equal(self.nodes[0].getreceivedbyaddress(a0), expected_bal)
assert_equal(self.nodes[0].getreceivedbyaddress(a1), expected_bal)

self.log.info("Test sendmany with fee_rate param (explicit fee rate in duff/B)")
fee_rate_sat_vb = 2
fee_rate_btc_kvb = fee_rate_sat_vb * 1e3 / 1e8
Expand Down
Loading