Skip to content

Conversation

@jamillambert
Copy link
Collaborator

The test was marked as TODO and the fields in the structs do not match what is returned by the RPC.

In v20 the return field labels changes from a vector of json objects that was modelled as GetAddressInfoLabel to a Vec<String>. To allow for this make the modelled type a Vec<String>, this means for v17-v19 the name field is modelled and the purpose field of GetAddressInfoLabel is not.

  • Fix all of the fields in v17.
  • Redefine the types for changes in v18, v29, v22 and v28.
  • Update the model for to allow for the changes in return types.
  • Update the test using different Address types to check the inner structs contain expected values.

/// The Hash160 of the HD seed.
pub hd_seed_id: Option<hash160::Hash>,
/// The fingerprint of the master key.
pub hd_master_fingerprint: Option<String>,
Copy link
Member

@tcharding tcharding Aug 5, 2025

Choose a reason for hiding this comment

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

The docs for this field are misleading, its only 4 bytes and is abstracted in rust-bbitcoin as bip32::Fingerprint.

bt18 getaddressinfo $(bt18 getnewaddress)
{
  "address": "2Mv8t4zo6LZNV7C6h8Yw4BPxDGmRhN2QgRc",
  "scriptPubKey": "a9141fb5411701d330094d66b47429fbd1e60cc5383087",
  "ismine": true,
  "solvable": true,
  "desc": "sh(wpkh([8b864bde/0'/0'/1']02183110dabe2b4fc707584013ecc609e341c379328ab1c5e15f5324fa188e1efa))#qymhf8xd",
  "iswatchonly": false,
  "isscript": true,
  "iswitness": false,
  "script": "witness_v0_keyhash",
  "hex": "00142046f772984248fcce2cc899e9c423e1c12f7451",
  "pubkey": "02183110dabe2b4fc707584013ecc609e341c379328ab1c5e15f5324fa188e1efa",
  "embedded": {
    "isscript": false,
    "iswitness": true,
    "witness_version": 0,
    "witness_program": "2046f772984248fcce2cc899e9c423e1c12f7451",
    "pubkey": "02183110dabe2b4fc707584013ecc609e341c379328ab1c5e15f5324fa188e1efa",
    "address": "bcrt1qypr0wu5cgfy0en3vezv7n3pru8qj7az3kr7cv0",
    "scriptPubKey": "00142046f772984248fcce2cc899e9c423e1c12f7451"
  },
  "label": "",
  "ischange": false,
  "timestamp": 1754432922,
  "hdkeypath": "m/0'/0'/1'",
  "hdseedid": "33b74e94e3adec8c7c8f1158098c89846ca29273",
  "hdmasterfingerprint": "8b864bde",
  "labels": [
    {
      "name": "",
      "purpose": "receive"
    }
  ]
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to bip32::Fingerprint and added the error variant.

Comment on lines 179 to 181
solvable: None, //v18 and above only.
descriptor: None, //v18 and above only.
parent_descriptor: None, //v22 and above only.
Copy link
Member

Choose a reason for hiding this comment

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

Typically code comments don't need to enforce too much style but since I'm the don in this crate I can get away with being even more anal than normal, can we have a space after the // please. Feel free to slap me :)

Copy link
Member

Choose a reason for hiding this comment

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

Here is a one-liner to fix it codebase wide (from Claude). I tested it works.

find . -name "*.rs" -type f -exec sed -i 's|//v|// v|g' {} +

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, worked for me too.

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.

Reviewed fc0a486

@jamillambert
Copy link
Collaborator Author

Suggested changes made. Also tidied up the imports in v18 types wallet.

The test was marked as TODO and the fields in the structs do not match
what is returned by the RPC.

Fix all of the fields in v17.

Redefine the types for changes in v18, v29, v22 and v28.  Update the
reexports.

Update the model for to allow for the changes in return types.

Update the error in v18 and reorganise wallet imports.

Update the test using different Address types to check the inner structs
contain expected values.
Reordering of reexports only.
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 6cc26f0

@tcharding tcharding merged commit d6873da into rust-bitcoin:master Aug 6, 2025
30 checks passed
blaze-smith470pm added a commit to blaze-smith470pm/corepc that referenced this pull request Sep 26, 2025
6cc26f076268d5d4a0350dd248ddd4d84d073e91 Run the formatter (Jamil Lambert, PhD)
b17157e818416bda964640dae603a9a3ab735b42 Fix getaddressinfo (Jamil Lambert, PhD)

Pull request description:

  The test was marked as TODO and the fields in the structs do not match what is returned by the RPC.

  In v20 the return field `labels` changes from a vector of json objects that was modelled as `GetAddressInfoLabel` to a `Vec<String>`. To allow for this make the modelled type a `Vec<String>`, this means for v17-v19 the `name` field is modelled and the  `purpose` field of `GetAddressInfoLabel` is not.

  - Fix all of the fields in v17.
  - Redefine the types for changes in v18, v29, v22 and v28.
  - Update the model for to allow for the changes in return types.
  - Update the test using different `Address` types to check the inner structs contain expected values.

ACKs for top commit:
  tcharding:
    ACK 6cc26f076268d5d4a0350dd248ddd4d84d073e91

Tree-SHA512: 52f371b06097e5b832edc1a2c0558329081b11b71cb9319fa72b0b157312ceda33b918d5fb0fb657e19225e1c705e09c84376d1c580d656f41ad7f8f4b6d4dd9
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