Skip to content

Conversation

@natobritto
Copy link
Contributor

@natobritto natobritto commented Jan 14, 2026

This PR:

  • Implements syncwithvalidationinterfacequeue and reconsiderblock in the v17 hidden module, add the client macro and test. Add the reexports for all later versions, as there are no changes in each up to v30.
  • Implements mockscheduler RPC starting from v20 to v30.
  • Corrects a small error in v30 mining comment.

Part of #333.

@tcharding
Copy link
Member

Thanks for your contribution! Just a quick warning before I review, this repo is quite strange. If you want more context to any of my review suggestions please ask.

() => {
impl Client {
pub fn sync_with_validation_interface_queue(&self) -> Result<()> {
self.call("syncwithvalidationinterfacequeue", &[])
Copy link
Member

Choose a reason for hiding this comment

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

For functions that return () we use this pattern

                match self.call("preciousblock", &[into_json(hash)?]) {
                    Ok(serde_json::Value::Null) => Ok(()),
                    Ok(res) => Err(Error::Returned(res.to_string())),
                    Err(err) => Err(err.into()),
                }

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: See comment below before actioning this

}

#[test]
fn blockchain__sync_with_validation_interface_queue__modelled() {
Copy link
Member

Choose a reason for hiding this comment

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

The __modelled suffix is for RCP methods that have a type defined in types::model.

Copy link
Member

Choose a reason for hiding this comment

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

Also this method is hidden, right? It should be tested in hidden.rs if so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, my oversight. As I have not defined any types, I'll remove the prefix and move tests to hidden.rs. Pushed a fix. Thanks for the review!

// Create activity that causes validation callbacks.
let (_address, _txid) = node.create_mempool_transaction();

let _: () = node
Copy link
Member

Choose a reason for hiding this comment

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

Oh interesting. I like this. It invalidates my previous suggestion, and we could use this pattern throughout the codebase instead of the 3 lines I posted to check for ().

Copy link
Member

Choose a reason for hiding this comment

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

@tcharding
Copy link
Member

Well done, clean. You managed to navigate this odd repository quite well. Thanks!

reviewed: 7756c42

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK bab4647

@tcharding tcharding merged commit 00e31f6 into rust-bitcoin:master Jan 15, 2026
30 checks passed
@tcharding
Copy link
Member

Thanks mate! Just for reference, usually I get folk to squash changes from review into the original patch instead of adding them at the end. Here I elected to just merge because you did a whole bunch of work already, thanks for the effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants