feat: schema: feature-gate deny_unknown_fields

[lvn-ryu]

This is a backwards-compatible change to the schema package which makes serde(deny_unknown_fields) optional.

It's feature-gated and not set by default.

cargo test --features allow-unknown-fields will run the new test_allow_unknown_fields().

I believe this test illustrates the use-case clearly in code, but a bit more explanation: this has been a real-world problem for us (Levana) - we have bots which are written in Rust and consume messages from contracts. They do share the same "message" library code, but oftentimes we need to stagger their deployment (e.g. deploy contracts with new fields for the frontend, and we do not want to immediately deploy new bots or indexer services). deny_unknown_fields causes the bots to then break, since they are unaware of the new fields - but they don't need or use those new fields at all. We want to allow it.

Tbh I'm a bit surprised this hasn't come up more in the wild, but that may be due to most clients using languages other than Rust which are not bound by the serde constraints (such as Typescript which does not error out on unknown fields.)

[webmaster128]

Thanks for bringing this up. I think the only reason deny_unknown_fields was ever introduced is the generated schema and the generated TypeScript types. At decoding level we don't even want the behaviour. I wonder if there is a better fix for it, e.g. by tweaking the schema/code generation only without affecting serde itself.

[lvn-ryu]

interesting... do you have more info for how deny_unknown_fields affects the schema generation? I'm not familiar with that - but if we could remove deny_unknown_fields altogether, that would be nicer, might be worth looking into :)

[webmaster128]

See #1307

[lvn-ryu]

oh wait - this PR isn't quite right... it requires that the caller have the feature, since it's just generated from the proc macro... not ready for merging yet, will ping for re-review

[lvn-ryu]

@webmaster128 - it's fixed and ready for review

It's a bit too time consuming for me to dig deeper on "try to get rid of deny_unknown_fields altogether" right now. What do you think about merging this in as a stop-gap, and opening a new tracking issue to link here if someone wants to pick it up and fix more holistically?

No pressure ofc, we can point at our fork for now if need-be (just for this, not any other package) :)

[webmaster128]

I was thinking about a different short term solution: Create an optional argument like this: #[cw_serde(deny_unknown_fields=false)]. This then is a non-breaking change we can release in CosmWasm 2.1. Changing behaviour with features is usually not great since it easily goes wrong in more complex setups, such as e.g. the macro being used in a library. Also the feature is not additive, which is a requirement for Cargo features.

[lvn-ryu]

Create an optional argument like this: #[cw_serde(deny_unknown_fields=false)]

sorry, not following what you mean there.. the compiler didn't like this when I tested it

Also the feature is not additive, which is a requirement for Cargo features

Ok, I've made it additive in 0837c36

to test now it's with cargo test --no-default-features

imho this is a bit harder to reason about since it has to control it by setting default-features = false in the dependency... I could imagine a future dev wondering what that's about - but not a biggie. If it lands with this approach, all good. If you'd like to close this PR and just note it as some sortof request for the future, also all good :)

[chipshort]

I'd also vote for the #[cw_serde(deny_unknown_fields=false)] version. Then we can add more arguments in the future and the change can be applied more granular.
The problem with using a feature here is that any transitive dependency enabling it will enable it for your crate, affecting all of your schemas.

sorry, not following what you mean there.. the compiler didn't like this when I tested it

Works fine for me:

#[cw_serde(deny_unknown_fields = false)]
pub struct State {
    pub count: i32,
}

Of course, at the moment it is just ignored.
What needs to be done here is to parse the attr param of the cw_serde macro, pass that into cw_serde_impl and check for the deny_unknown_fields value

[lvn-ryu]

ahhh, I see what you're saying now. Yes, I agree, that's much nicer :)

[lvn-ryu]

I'm going to go ahead and close this PR... not sure if I'll get around to opening one with that other approach, but if so, I'll link here for reference

[lvn-ryu]

I'll open an issue to track it though