test: Add multiple transactions and error handling tests for getreceivedbyaddress #34188

b-l-u-e · 2026-01-02T05:56:38Z

This PR adds comprehensive functional test coverage for the getreceivedbyaddress RPC method.

bensig · 2026-01-02T08:18:12Z

ACK 0d0b34d

Built with wallet support and ran on macOS - all 4 test scenarios pass.

Good coverage of minconf behavior, multiple transactions, error handling, and coinbase maturity.

nit: Copyright header says 2017-2022 but this is a new file - might want to update to 2026-present or similar.

b-l-u-e · 2026-01-02T08:51:47Z

nit: Copyright header says 2017-2022 but this is a new file - might want to update to 2026-present or similar.

Aah thank you I will update

DrahtBot · 2026-01-02T08:51:51Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34188.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fjahr
Concept ACK	rkrux
Stale ACK	bensig

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30972 (Wallet: "listreceivedby*" fix by BrandonOdiwuor)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2026-01-02T08:53:26Z

How is this different from test/functional/wallet_listreceivedby.py? Can you explain what exact coverage is missing and what exact coverage is added?

b-l-u-e · 2026-01-02T10:38:40Z

How is this different from test/functional/wallet_listreceivedby.py? Can you explain what exact coverage is missing and what exact coverage is added?

the test adds two important coverage areas that are missing in wallet_listreceivedby.py

multiple transactions to the same address , which is a use case not covered in the existing test. The existing test only sends a single transaction (0.1 BTC) to an address.

invalid address format error - tests error -5 "Invalid Bitcoin address" for malformed address strings. The existing test only covers error -4 "Address not found in wallet" this completes the error handling coverage.

test file provides better organization and focuses specifically on getreceivedbyaddress , there is some overlap with basic functionality, but the new test adds valuable coverage that was missing.

maflcko · 2026-01-02T11:28:03Z

My preference would be to keep related functionality checks for related RPCs in the same file, not create a separate file for each RPC. This will also avoid checking the same overlap several times redundantly.

fanquake · 2026-01-02T15:12:27Z

https://github.com/bitcoin/bitcoin/actions/runs/20658978454/job/59322217188?pr=34188#step:6:145:

 The subject line of commit hash 1538fc6d86d1d439646ab724ca7a721867213b14 is followed by a non-empty line. Subject lines should always be followed by a blank line.

DrahtBot · 2026-01-02T16:26:10Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20658978454/job/59322217188}
_{LLM reason (✨ experimental): Commit message formatting failed lint check (missing blank line after the subject), causing the CI to fail.}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

test/functional/wallet_listreceivedby.py

rkrux

Concept ACK 66bd4bf

https://maflcko.github.io/b-c-cov/total.coverage/src/wallet/rpc/coins.cpp.gcov.html

Although there isn't increased coverage due to the multiple transactions on same address test but logically it's good to have such a test. The invalid address test should increase the coverage.

test/functional/wallet_listreceivedby.py

fanquake · 2026-01-09T16:31:11Z

https://github.com/bitcoin/bitcoin/actions/runs/20722345957/job/59492260670?pr=34188#step:14:221:

Traceback (most recent call last):
  File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 142, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "D:\a\bitcoin\bitcoin\build\test\functional\feature_bip68_sequence.py", line 66, in run_test
    self.test_sequence_lock_confirmed_inputs()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "D:\a\bitcoin\bitcoin\build\test\functional\feature_bip68_sequence.py", line 149, in test_sequence_lock_confirmed_inputs
    num_inputs = random.randint(1, min(10, available_utxos))
  File "C:\hostedtoolcache\windows\Python\3.14.2\x64\Lib\random.py", line 341, in randint
    raise ValueError(f"empty range in randint({a}, {b})")
ValueError: empty range in randint(1, 0)

b-l-u-e · 2026-01-15T09:04:39Z

https://github.com/bitcoin/bitcoin/actions/runs/20722345957/job/59492260670?pr=34188#step:14:221:

Traceback (most recent call last):
  File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 142, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "D:\a\bitcoin\bitcoin\build\test\functional\feature_bip68_sequence.py", line 66, in run_test
    self.test_sequence_lock_confirmed_inputs()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "D:\a\bitcoin\bitcoin\build\test\functional\feature_bip68_sequence.py", line 149, in test_sequence_lock_confirmed_inputs
    num_inputs = random.randint(1, min(10, available_utxos))
  File "C:\hostedtoolcache\windows\Python\3.14.2\x64\Lib\random.py", line 341, in randint
    raise ValueError(f"empty range in randint({a}, {b})")
ValueError: empty range in randint(1, 0)

the issue has been fixed in #34244. I rebased the PR.

fjahr · 2026-01-15T13:36:44Z

the issue has been fixed in #34244. I rebased the PR.

Just FYI, since this was an unrelated test failure in a file you are not touching, in the future you can either ask for someone to rerun the CI job for you or you can push a rebase proactively to retrigger the CI and see if the error really persists.

fjahr · 2026-01-15T13:39:22Z

But now there is a CI failure that seems related:

 test  2026-01-15T10:18:10.109856Z TestFramework (ERROR): Unexpected exception 
                                   Traceback (most recent call last):
                                     File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/test_framework.py", line 142, in main
                                       self.run_test()
                                     File "/home/admin/actions-runner/_work/_temp/build/test/functional/wallet_listreceivedby.py", line 133, in run_test
                                       assert_equal(balance, Decimal("0.3"))
                                     File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/util.py", line 80, in assert_equal
                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
                                   AssertionError: not(0.10000000 == 0.3)

b-l-u-e · 2026-01-15T15:12:16Z

But now there is a CI failure that seems related:

 test  2026-01-15T10:18:10.109856Z TestFramework (ERROR): Unexpected exception 
                                   Traceback (most recent call last):
                                     File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/test_framework.py", line 142, in main
                                       self.run_test()
                                     File "/home/admin/actions-runner/_work/_temp/build/test/functional/wallet_listreceivedby.py", line 133, in run_test
                                       assert_equal(balance, Decimal("0.3"))
                                     File "/home/admin/actions-runner/_work/_temp/test/functional/test_framework/util.py", line 80, in assert_equal
                                       raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
                                   AssertionError: not(0.10000000 == 0.3)

CI failed coz without self.sync_all() node 1 didn't have both transactions in its mempool when blocks were generated.
so only the first transaction (0.1 BTC) was included causing getreceivedbyaddress to return 0.1 instead of 0.3. so i added back self.sync_all() after both transactions to ensure theres proper propagation before block generation

fjahr

utACK fa45a3b

Could make minor improvements to the sync but this looks ok for me as well.

test/functional/wallet_listreceivedby.py

- Add test for multiple transactions to same address - Add test for invalid address format error

fjahr · 2026-01-15T21:34:33Z

utACK dec4887

Just addressed my comment since last review.

maflcko · 2026-01-16T15:02:02Z

test/functional/wallet_listreceivedby.py

+        addr_with_multiple_txs = self.nodes[1].getnewaddress()
+        self.nodes[0].sendtoaddress(addr_with_multiple_txs, Decimal("0.1"))
+        self.nodes[0].sendtoaddress(addr_with_multiple_txs, Decimal("0.2"))
+        self.generate(self.nodes[0], 10)


why generate 10 blocks, when 1 would be sufficient?

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from 0d0b34d to e7fb202 Compare January 2, 2026 10:03

DrahtBot added CI failed Tests and removed CI failed labels Jan 2, 2026

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from e7fb202 to 1538fc6 Compare January 2, 2026 13:34

b-l-u-e changed the title ~~test: Add functional test for getreceivedbyaddress RPC~~ test: Add multiple transactions and error handling tests for getreceivedbyaddress Jan 2, 2026

DrahtBot added the CI failed label Jan 2, 2026

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from 1538fc6 to 9944a4c Compare January 2, 2026 18:32

DrahtBot mentioned this pull request Jan 3, 2026

Wallet: "listreceivedby*" fix #30972

Open

fjahr reviewed Jan 3, 2026

View reviewed changes

test/functional/wallet_listreceivedby.py Outdated Show resolved Hide resolved

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from 9944a4c to 66bd4bf Compare January 4, 2026 12:50

b-l-u-e requested a review from fjahr January 4, 2026 12:56

rkrux suggested changes Jan 5, 2026

View reviewed changes

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from 66bd4bf to 0cbef7c Compare January 5, 2026 16:42

b-l-u-e requested a review from rkrux January 5, 2026 16:47

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from 0cbef7c to bfb2642 Compare January 15, 2026 09:01

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from bfb2642 to fa45a3b Compare January 15, 2026 14:47

fjahr reviewed Jan 15, 2026

View reviewed changes

test/functional/wallet_listreceivedby.py Outdated Show resolved Hide resolved

test: Add getreceivedbyaddress coverage to wallet_listreceivedby

dec4887

- Add test for multiple transactions to same address - Add test for invalid address format error

b-l-u-e force-pushed the test-wallet-getreceivedbyaddress branch from fa45a3b to dec4887 Compare January 15, 2026 18:38

DrahtBot removed the CI failed label Jan 15, 2026

maflcko reviewed Jan 16, 2026

View reviewed changes

test: Add multiple transactions and error handling tests for getreceivedbyaddress #34188

Are you sure you want to change the base?

test: Add multiple transactions and error handling tests for getreceivedbyaddress #34188

Conversation

b-l-u-e commented Jan 2, 2026

Uh oh!

bensig commented Jan 2, 2026

Uh oh!

b-l-u-e commented Jan 2, 2026

Uh oh!

DrahtBot commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

maflcko commented Jan 2, 2026

Uh oh!

b-l-u-e commented Jan 2, 2026

Uh oh!

maflcko commented Jan 2, 2026

Uh oh!

fanquake commented Jan 2, 2026

Uh oh!

DrahtBot commented Jan 2, 2026

Uh oh!

Uh oh!

rkrux left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fanquake commented Jan 9, 2026

Uh oh!

b-l-u-e commented Jan 15, 2026

Uh oh!

fjahr commented Jan 15, 2026

Uh oh!

fjahr commented Jan 15, 2026

Uh oh!

b-l-u-e commented Jan 15, 2026

Uh oh!

fjahr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fjahr commented Jan 15, 2026

Uh oh!

maflcko Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

DrahtBot commented Jan 2, 2026 •

edited

Loading