Add `DecodeScriptSegwit` Parsing and Model Support for `decodescript` RPC `Segwit` Field #290

Her-Code · 2025-07-11T09:16:54Z

Summary

This PR addresses #270
It adds parsing logic and model representation for the segwit field returned by the decodescript RPC, which was previously ignored.

Changes

Introduce DecodeScriptSegwit struct to model segwit data.
Update DecodeScript to include an optional segwit field.
Add DecodeScriptSegwitError for field-level error handling when parsing segwit.
Re-export DecodeScriptSegwit and DecodeScriptSegwitError in v22, v23, and mod.rs.
Set segwit to None for Core versions that don’t support it.

jamillambert

Thanks for having a look into this. A few pointers, some are only minor but might be useful for you to improve going forward.

The commit log could be improved. Should start with a capital. You aren't actually adding segwit support, you are adding a model to convert the returned data from the RPC into rust-bitcoin types. I have been told to write commit logs in the imperative (like you are telling the commit what to do), so also suggesting it to you. i.e. Add struct, not added, adding etc. It fits with what git does when generating messages, and makes reading them easier if everyone does the same thing.

The general comment though is have a look at other RPCs in this crate, it is all very structured with the same format, comment style etc. and any new additions should follow this. If you're using AI, be more specific in what you want and give it some files to look at as a reference.

jamillambert · 2025-07-11T13:48:03Z

types/src/model/raw_transactions.rs

+pub struct DecodeScriptSegwit {
+    pub asm: String,
+    pub hex: String,
+    #[serde(rename = "type")]
+    pub type_: String,
+    pub address: Option<bitcoin::Address<bitcoin::address::NetworkUnchecked>>,
+    #[serde(rename = "reqSigs")]
+    pub required_signatures: Option<u64>,
+    pub addresses: Vec<bitcoin::Address<bitcoin::address::NetworkUnchecked>>,
+    #[serde(rename = "p2sh-segwit")]
+    pub p2sh_segtwit: Option<bitcoin::Address<bitcoin::address::NetworkUnchecked>>,
+}


All of the structs in this crate have a specific format. Have a look at the others, e.g. the one above, and follow what is done there including comments, type declarations, use of serde rename in types and not model.

jamillambert · 2025-07-11T13:54:47Z

types/src/v22/raw_transactions/into.rs

+        let segwit = self.segwit.map(|s| DecodeScriptSegwit {
+            asm: s.asm,
+            hex: s.hex,
+            type_: s.type_,
+            address: s.address,
+            required_signatures: s.required_signatures,
+            addresses: s.addresses,
+            p2sh_segtwit: s.p2sh_segtwit,


This should be implemented as an into_model function for the DecodeScriptSegwit type, and all future versions should import and use it unless there is a change in the RPC. Have a look at another RPC that returns a json object that is modelled like GetTransaction and it's sub type GetTransactionDetail in v20 wallet.

jamillambert

Getting better but still some things to work on.

Can you also update the PR title and description, you are not adding segwit support.

jamillambert · 2025-07-14T14:37:59Z

types/src/v22/raw_transactions/error.rs

+#[derive(Debug)]
+pub enum DecodeScriptSegwitError {
+    /// An invalid or missing field in the `DecodeScriptSegwit` data.
+    InvalidField(&'static str),


I don't understand this error variant or why it exists?

jamillambert · 2025-07-14T16:35:31Z

types/src/v22/mod.rs

@@ -253,7 +253,7 @@ pub use self::{
    blockchain::GetMempoolInfo,
    control::Logging,
    network::{Banned, GetPeerInfo, ListBanned},
-    raw_transactions::{DecodeScript, DecodeScriptError},
+    raw_transactions::{DecodeScript, DecodeScriptError,DecodeScriptSegwit,DecodeScriptSegwitError},


Suggested change

raw_transactions::{DecodeScript, DecodeScriptError,DecodeScriptSegwit,DecodeScriptSegwitError},

raw_transactions::{DecodeScript, DecodeScriptError, DecodeScriptSegwit, DecodeScriptSegwitError},

There are a number of formatting issues like this one. There is a script that will fix most of them just fmt it runs cargo fmt on the current nightly for the crate stored in nightly-version, which is not the most recent nightly. There are other helpful scripts in the justfile that are worth a look, there have been some changes in the rust nightly recently and the most recent nightly might give a few lint errors.

You should also be able to configure your editor to remove trailing white spaces. There are quite a few in this PR.

types/src/model/raw_transactions.rs

jamillambert · 2025-07-14T17:03:53Z

types/src/model/raw_transactions.rs

+    /// List of bitcoin addresses.
+    pub addresses: Vec<Address<NetworkUnchecked>>,
+    /// Address of the P2SH script wrapping this witness redeem script.
+    pub p2sh_segwit: Option<String>,


This should be Address<NetworkUnchecked> not String. I think the same should also be done for DecodeScript.

jamillambert

PR title still needs to be changed.

Can you squash the commits? and if you are still going to do the changes to existing docs can you do this in a separate commit.

The next step is to add a test, or update the existing test. And add all the reexports.

jamillambert · 2025-07-15T10:53:28Z

types/src/v23/raw_transactions/error.rs

+
+/// Error when converting a `DecodeScriptSegwit` type into the model type.
+#[derive(Debug)]
+pub enum DecodeScriptSegwitError {


This is not needed, it is identical the the exiting one. Import the previous version one and use that.

jamillambert · 2025-07-15T10:59:42Z

types/src/model/raw_transactions.rs

+    /// The raw output script bytes, hex-encoded.
+    pub hex: ScriptBuf,
+    /// Inferred descriptor for the script. v23 and later only.
+    pub descriptor: Option<String>,


descriptor should be where it appears in the list of the help, i.e. second last.

jamillambert · 2025-07-15T11:00:05Z

types/src/v22/raw_transactions/error.rs

+
+/// Error when converting a `DecodeScriptSegwit` type into the model type.
+#[derive(Debug)]
+pub enum DecodeScriptSegwitError {


This should be in v17

jamillambert · 2025-07-15T11:00:19Z

types/src/v22/raw_transactions/into.rs

            p2sh_segwit: self.p2sh_segwit,
        })
    }
 }
+
+impl DecodeScriptSegwit {


This is needed in v17 as well

jamillambert · 2025-07-15T11:04:55Z

types/src/v17/raw_transactions/into.rs

@@ -310,6 +310,7 @@ impl DecodeScript {
            addresses,
            p2sh,
            p2sh_segwit: self.p2sh_segwit,
+            segwit: None,


I think this is returned in v17 and just not documented until v19

jamillambert · 2025-07-15T11:08:19Z

types/src/v22/raw_transactions/mod.rs

@@ -23,9 +25,9 @@ pub use self::error::DecodeScriptError;
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
 #[serde(deny_unknown_fields)]
 pub struct DecodeScript {


All the docs changes in DecodeScript are not related to adding the DecodeScriptSegwit model. They appear to have been changed from what one version outputs to another's. I would just leave them as they were, or if they are wrong you can change them but do it in a separate commit.

- Add `DecodeScriptSegwit` struct to model the `segwit` field returned by the `decodescript` RPC. - Update `DecodeScript` to include an optional `segwit` field. Add DecodeScriptSegwit struct, conversions, and model support - Add `DecodeScriptSegwit` struct to both versioned and model representations. - Implement `into_model()` for `DecodeScriptSegwit` and update `DecodeScript` accordingly. - Use `ScriptBuf` instead of `String` for `hex` to strongly type the field. - Replace `String` with `Address<NetworkUnchecked>` for `p2sh_segwit` and other fields. - Normalize and correct field comments to match Core `decodescript` RPC output. - Clean up formatting errors Add DecodeScriptSegwit into_model to v17 and refactor error handling - Add `into_model` implementation for `DecodeScriptSegwit` in v17. - Return `segwit` in v17, as it is present in RPC output despite not being documented until v19. - Add `DecodeScriptSegwitError` enum in v17, as `address` is sometimes `None` and error handling is needed. - Remove duplicate `DecodeScriptSegwitError` from v23 and reuse the one from v22 via import. - Move `descriptor` field in `DecodeScriptSegwit` model struct to match the field order in Bitcoin Core's `decodescript` RPC response.

github-actions bot added the C-types label Jul 11, 2025

jamillambert reviewed Jul 11, 2025

View reviewed changes

Her-Code force-pushed the model-for-segwit branch 3 times, most recently from 448feed to c9964af Compare July 14, 2025 10:35

Her-Code requested a review from jamillambert July 14, 2025 10:52

Her-Code force-pushed the model-for-segwit branch from c9964af to 9a10ba4 Compare July 14, 2025 11:07

jamillambert reviewed Jul 14, 2025

View reviewed changes

Her-Code requested a review from jamillambert July 15, 2025 09:35

jamillambert reviewed Jul 15, 2025

View reviewed changes

Her-Code changed the title ~~Add segwit support to DecodeScript model and conversions~~ Add DecodeScriptSegwit Parsing and Model Support for decodescript RPC Segwit Field Jul 16, 2025

Her-Code requested a review from jamillambert July 16, 2025 12:35

Her-Code force-pushed the model-for-segwit branch from a9d3723 to adbb6a0 Compare July 17, 2025 15:27

Add model test for decode_script with P2WPKH SegWit output

d17f7a9

github-actions bot added the C-integration-test label Jul 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `DecodeScriptSegwit` Parsing and Model Support for `decodescript` RPC `Segwit` Field #290

Add `DecodeScriptSegwit` Parsing and Model Support for `decodescript` RPC `Segwit` Field #290

Uh oh!

Her-Code commented Jul 11, 2025 •

edited

Loading

Uh oh!

jamillambert left a comment

Uh oh!

jamillambert Jul 11, 2025

Uh oh!

jamillambert Jul 11, 2025

Uh oh!

jamillambert left a comment •

edited

Loading

Uh oh!

jamillambert Jul 14, 2025

Uh oh!

jamillambert Jul 14, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

jamillambert Jul 14, 2025

Uh oh!

jamillambert left a comment •

edited

Loading

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

jamillambert Jul 15, 2025

Uh oh!

Uh oh!

	raw_transactions::{DecodeScript, DecodeScriptError,DecodeScriptSegwit,DecodeScriptSegwitError},
	raw_transactions::{DecodeScript, DecodeScriptError, DecodeScriptSegwit, DecodeScriptSegwitError},

Add DecodeScriptSegwit Parsing and Model Support for decodescript RPC Segwit Field #290

Are you sure you want to change the base?

Add DecodeScriptSegwit Parsing and Model Support for decodescript RPC Segwit Field #290

Uh oh!

Conversation

Her-Code commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

jamillambert left a comment

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamillambert Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamillambert left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Add `DecodeScriptSegwit` Parsing and Model Support for `decodescript` RPC `Segwit` Field #290

Add `DecodeScriptSegwit` Parsing and Model Support for `decodescript` RPC `Segwit` Field #290

Her-Code commented Jul 11, 2025 •

edited

Loading

jamillambert left a comment •

edited

Loading

jamillambert Jul 14, 2025 •

edited

Loading

jamillambert left a comment •

edited

Loading