Add DistributionQuery::DelegatorWithdrawAddress
#442
Conversation
| } | ||
|
|
||
| type DelegatorWithdrawAddressResponse struct { | ||
| WithdrawAddress string `json:"withdraw_address"` |
There was a problem hiding this comment.
I just realize in cosmwasm (Rust) this should be Addr instead of String because we get a checked value from wasmd here.
There was a problem hiding this comment.
In that case we cannot have the Default derive for the DelegatorWithdrawAddressResponse and therefore also not the QueryResponseType impl. Is that alright? I think we need to add a constructor function then, if we want to mock it in multi-test.
There was a problem hiding this comment.
Uff, yeah, this problem again. We hit the exact same case with ContractInfoResponse. In essence: in normale cases a contract never constructs this and just consumes it. Just for testing frameworks we need a constructor outside of cosmwasm-std which means we need to either have Default or give up #[non_exhaustive]. Having Default allows constructing invalid instances. I don't like this situation but don't have a solution.
Happy for any solution suggestion to this general problem but let's not block the release on it.
There was a problem hiding this comment.
I suppose in this case we could just drop the non_exhaustive, as I do not think another field will be added to this response in particular.
I have some ideas on how to solve this in general:
- keep separate Response structs in multi-test (as they only need to deserialize to the same), but then we have two separate ones, which is very annoying.
- add a public function to create the response, but with
#[doc(hidden)]. This has similar drawbacks as theDefaultimpl, except that we can demand parameters and clearly mark it as not for "normal" usage.
There was a problem hiding this comment.
The second option means a non-stable constructor which I guess is fine. The other option between the two is a stable constructor which sets the newly added fields to some default value.
There was a problem hiding this comment.
Yes, a non-stable constructor or default values if the added fields allow for one, but realistically non-stable constructor should be fine if we hide it and clearly document it to be non-stable.
There was a problem hiding this comment.
Could you add those constructors for all query responses? Then we can remove Default here and make it Addr. I think we should favour the production use case over test framework convenience. And having a validated Addr save a call into the chain.
closes #436
Implements the query types for CosmWasm/cosmwasm#1593