Add missing v18 wallet methods #237

jamillambert · 2025-06-11T20:10:41Z

Add getreceivedbylabel, listreceivedbylabel and getdescriptorinfo methods, models and tests.

tcharding

In general I don't think we should have arguments to client functions that are optional, unless we need them for some test. E.g I hacked impl_client_v18__list_received_by_label and was able to remove all three args.

client/src/client_sync/v27/mod.rs

tcharding · 2025-06-12T02:47:03Z

integration_test/tests/wallet.rs

+    let node = Node::with_wallet(Wallet::Default, &[]);
+    node.fund_wallet();
+    let label = "test-label";
+    let _ = node.client.new_address_with_label(label).unwrap();
+
+    // Send some coins to the label
+    let amount = Amount::from_sat(10_000);
+    let address = node.client.new_address_with_label(label).unwrap();
+    let address = address.assume_checked();
+    let _ = node.client.send_to_address(&address, amount).unwrap();
+    node.mine_a_block();
+
+    let json: ListReceivedByLabel = node.client.list_received_by_label(None, None, None).expect("listreceivedbylabel");
+    let model: Result<mtype::ListReceivedByLabel, ListReceivedByLabelError> = json.into_model();
+    let model = model.unwrap();
+    assert!(model.0.iter().any(|item| item.label == label));


Perhaps this cleaner?

let node = Node::with_wallet(Wallet::Default, &[]); node.fund_wallet(); let label = "test-label"; // Send some coins to the label let amount = Amount::from_sat(10_000); let address = node.client.new_address_with_label(label).unwrap().assume_checked(); let _ = node.client.send_to_address(&address, amount).unwrap(); node.mine_a_block(); let json: ListReceivedByLabel = node.client.list_received_by_label().expect("listreceivedbylabel"); let model: Result<mtype::ListReceivedByLabel, ListReceivedByLabelError> = json.into_model(); let model = model.unwrap(); assert!(model.0.iter().any(|item| item.label == label));

Thanks, changed.

tcharding · 2025-06-12T02:48:53Z

types/src/model/wallet.rs

+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
+pub struct ListReceivedByLabelItem {
+    /// Only returned if imported addresses were involved in transaction.
+    pub involves_watchonly: Option<bool>,


Suggested change

pub involves_watchonly: Option<bool>,

pub involves_watch_only: Option<bool>,

I was unsure of this but we have one instance already: is_watch_only. If you feel strongly you could change them both.

Yeah core is inconsistent e.g. as one word This functionality is only intended for use with non-watchonly addresses., but also with a dash enforced watch-only wallet. And also as involvesWatchonly even though all other RPC return variables are all lowercase.

I think with an _ fits better with the rest of the crate, so I changed it as suggested.

Yeah we shouldn't follow too closely what Core does re word splitting, we have a totally different philosophy here.

tcharding · 2025-06-12T20:19:37Z

integration_test/tests/wallet.rs

+    let node = Node::with_wallet(Wallet::Default, &[]);
+    node.fund_wallet();
+    let label = "test-label";
+    let _ = node.client.new_address_with_label(label).unwrap();


Why make this call?

It was from a previous version of the test, removed now.

tcharding · 2025-06-12T20:20:01Z

integration_test/tests/wallet.rs

+    let address = node.client.new_address_with_label(label).unwrap();
+    let address = address.assume_checked();


Suggested change

let address = node.client.new_address_with_label(label).unwrap();

let address = address.assume_checked();

let address = node.client.new_address_with_label(label).unwrap().assume_checked();

tcharding · 2025-06-12T20:20:32Z

integration_test/tests/wallet.rs

+    let model = model.unwrap();
+    assert!(model.0 == amount);


Suggested change

let model = model.unwrap();

assert!(model.0 == amount);

assert!(model.unwrap().0 == amount);

I changed it to assert_eq! too, the assert! was from a previous iteration that was >= amount in case there were already funds before the send to address.

tcharding · 2025-06-12T20:21:47Z

integration_test/tests/wallet.rs

+    let model: Result<mtype::ListReceivedByLabel, ListReceivedByLabelError> = json.into_model();
+    let model = model.unwrap();
+    assert!(model.0.iter().any(|item| item.label == label));
+}


Oh, you used my suggestion here but didn't follow along in wallet__get_received_by_label__modelled, my bad I thought you'd notice that.

I got thoroughly confused just then, I swear I changed this, but then realised that it was wallet__list_received_by_label__modelled. They are so similar I think my brain just merged them into one. Definitely my bad I missed it.

No stress, I got confused too actually when checking it.

Add `getreceivedbylabel`, `listreceivedbylabel` and `getdescriptorinfo` methods, models and tests.

Reorganising the reexports alphabetically, no other changes.

jamillambert

Suggested changes all made

tcharding

ACK ef5dda8

github-actions bot added C-client C-integration-test C-types labels Jun 11, 2025

jamillambert force-pushed the 0611-v18-wallet branch 2 times, most recently from b760893 to be1d9b3 Compare June 11, 2025 20:42

tcharding mentioned this pull request Jun 12, 2025

types: model types shouldn't have signed values unless its meaningful - I think #238

Open

tcharding reviewed Jun 12, 2025

View reviewed changes

jamillambert force-pushed the 0611-v18-wallet branch from be1d9b3 to cd60513 Compare June 12, 2025 07:20

tcharding reviewed Jun 12, 2025

View reviewed changes

jamillambert added 2 commits June 13, 2025 08:27

Add missing v18 wallet methods

46760cd

Add `getreceivedbylabel`, `listreceivedbylabel` and `getdescriptorinfo` methods, models and tests.

Run the formatter

ef5dda8

Reorganising the reexports alphabetically, no other changes.

jamillambert commented Jun 13, 2025

View reviewed changes

jamillambert force-pushed the 0611-v18-wallet branch from cd60513 to ef5dda8 Compare June 13, 2025 07:30

tcharding mentioned this pull request Jun 13, 2025

CI: We are not checking lint warnings #246

Open

tcharding approved these changes Jun 13, 2025

View reviewed changes

tcharding merged commit 1a524eb into rust-bitcoin:master Jun 13, 2025
29 checks passed

	pub involves_watchonly: Option<bool>,
	pub involves_watch_only: Option<bool>,

		let address = node.client.new_address_with_label(label).unwrap();
		let address = address.assume_checked();

	let model = model.unwrap();
	assert!(model.0 == amount);
	assert!(model.unwrap().0 == amount);

Add missing v18 wallet methods #237

Add missing v18 wallet methods #237

Uh oh!

Conversation

jamillambert commented Jun 11, 2025

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamillambert left a comment

Choose a reason for hiding this comment

Uh oh!

tcharding left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!