Skip to content

Add a feature for serde deny unknown fields #367

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 24, 2025
Merged

Add a feature for serde deny unknown fields #367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8fa8d6
Reorder dependencies in Cargo.toml files
jamillambert Sep 23, 2025
ced1f4f
Use relative path dependencies for corepc crates
jamillambert Sep 22, 2025
2406d12
Remove serde deny_unknown_fields from client
jamillambert Sep 22, 2025
ba366fd
Add a feature for serde deny unknown fields
jamillambert Sep 18, 2025
14c3dd6
Feature gate serde(deny_unknown_fields)
jamillambert Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
[workspace]
members = ["client", "types", "node", "jsonrpc"]
members = ["client", "jsonrpc", "node", "types"]
exclude = ["integration_test", "verify"]
resolver = "2"

[patch.crates-io.corepc-client]
path = "client"

[patch.crates-io.corepc-types]
path = "types"
[patch.crates-io.jsonrpc]
path = "jsonrpc"

[patch.crates-io.corepc-node]
path = "node"

[patch.crates-io.jsonrpc]
path = "jsonrpc"
[patch.crates-io.corepc-types]
path = "types"
4 changes: 2 additions & 2 deletions client/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ client-sync = ["jsonrpc"]

[dependencies]
bitcoin = { version = "0.32.0", default-features = false, features = ["std", "serde"] }
types = { package = "corepc-types", version = "0.9.0", default-features = false, features = ["std"] }
log = "0.4"
serde = { version = "1.0.103", default-features = false, features = [ "derive", "alloc" ] }
serde_json = { version = "1.0.117" }
types = { package = "corepc-types", path = "../types", default-features = false, features = ["std"] }

jsonrpc = { version = "0.18.0", features = ["minreq_http"], optional = true }
jsonrpc = { path = "../jsonrpc", features = ["minreq_http"], optional = true }

[dev-dependencies]
1 change: 0 additions & 1 deletion client/src/client_sync/v21/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ crate::impl_client_v17__get_zmq_notifications!();

/// Request object for the `importdescriptors` method.
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
pub struct ImportDescriptorsRequest {
/// Descriptor to import.
#[serde(rename = "desc")]
Expand Down
15 changes: 4 additions & 11 deletions integration_test/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,10 @@ TODO = [] # This is a dirty hack while writing the tests.

[dependencies]
bitcoin = { version = "0.32.0", default-features = false, features = ["std", "serde"] }
node = { package = "corepc-node", version = "0.9.0", default-features = false }
rand = "0.8.5"
env_logger = "0.9.0"
node = { package = "corepc-node", path = "../node", default-features = false }
rand = "0.8.5"
# Just so we can enable the feature.
types = { package = "corepc-types", path = "../types", features = ["serde-deny-unknown-fields"] }

[dev-dependencies]

[patch.crates-io.corepc-client]
path = "../client"

[patch.crates-io.corepc-types]
path = "../types"

[patch.crates-io.corepc-node]
path = "../node"
10 changes: 5 additions & 5 deletions node/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ rust-version = "1.63.0"
exclude = ["tests", "contrib"]

[dependencies]
corepc-client = { version = "0.9.0", features = ["client-sync"] }
log = { version = "0.4", default-features = false }
which = { version = "3.1.1", default-features = false }
anyhow = { version = "1.0.66", default-features = false, features = ["std"] }
tempfile = {version = "3", default-features = false }
corepc-client = { path = "../client", features = ["client-sync"] }
log = { version = "0.4", default-features = false }
serde_json = { version = "1.0.117", default-features = false }
tempfile = { version = "3", default-features = false }
which = { version = "3.1.1", default-features = false }

[dev-dependencies]
env_logger = { version = "0.9.3", default-features = false }
Expand All @@ -27,8 +27,8 @@ env_logger = { version = "0.9.3", default-features = false }
anyhow = { version = "1.0.66", optional = true }
bitcoin_hashes = { version = ">= 0.13, <= 0.14", optional = true }
flate2 = { version = "1.0", optional = true }
tar = { version = "0.4", optional = true }
minreq = { version = "2.9.1", default-features = false, features = ["https"], optional = true }
tar = { version = "0.4", optional = true }
zip = { version = "0.6.6", default-features = false, features = ["bzip2", "deflate"], optional = true }

# Please note, it is expected that a single version feature will be enabled however if you enable
Expand Down
1 change: 1 addition & 0 deletions types/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ exclude = ["tests", "contrib"]
[features]
default = ["std"]
std = ["bitcoin/std"]
serde-deny-unknown-fields = []
Copy link
Contributor

@tnull tnull Sep 22, 2025

Choose a reason for hiding this comment

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

To maintain the previous default behavior, would it make sense to add this to default here and in the other crates?

Copy link
Collaborator Author

@jamillambert jamillambert Sep 22, 2025

Choose a reason for hiding this comment

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

We decided to have it off by default. The feature is only needed in testing, see the comment above by @tcharding.

Copy link
Member

@tcharding tcharding Sep 22, 2025

Choose a reason for hiding this comment

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

Thanks for reviewing @tnull! Good to see you.

Copy link
Contributor

@tnull tnull Sep 23, 2025

Choose a reason for hiding this comment

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

I don't quite follow why it's only used in tests, but also don't have a super strong opinion that it needs to be enabled by default.

Copy link
Collaborator Author

@jamillambert jamillambert Sep 23, 2025

Choose a reason for hiding this comment

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

I don't really have a strong opinion either and did have it on by default to start with.

The reason though is that with the feature off the decoding will pass even if there is a returned field not in the struct, which means if there are changes in Core the current functionality of types will remain the same. All the returned fields from the RPCs in their current state will still be decoded, otherwise it will stop working altogether until it is patched.

Obviously we still have the feature on in integration_test so that any changes to the returned fields will be caught in testing, which is the reason it was added in the first place.


[dependencies]
bitcoin = { version = "0.32.0", default-features = false, features = ["serde", "base64", "secp-recovery"] }
Expand Down
4 changes: 2 additions & 2 deletions types/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub fn compact_size_decode(slice: &mut &[u8]) -> u64 {
/// backwards compatible so we only provide it not a v0.17 specific type. The `mtype::ScriptPubkey`
/// mirrors this design (but with concrete `rust-bitcoin` types).
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
#[cfg_attr(feature = "serde-deny-unknown-fields", serde(deny_unknown_fields))]
pub struct ScriptPubkey {
/// Script assembly.
pub asm: String,
Expand Down Expand Up @@ -219,7 +219,7 @@ impl ScriptPubkey {

/// Data returned by Core for a script signature.
#[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
#[cfg_attr(feature = "serde-deny-unknown-fields", serde(deny_unknown_fields))]
pub struct ScriptSig {
/// Assembly representation of the script.
pub asm: String,
Expand Down
Loading