Split StakingMsg::Withdraw #876
Conversation
ethanfrey
left a comment
ethanfrey
left a comment
There was a problem hiding this comment.
Looks good, nice comments.
I would just like to ensure these hide behind the staking feature flag
| }, | ||
| } | ||
|
|
||
| /// The message types of the distribution module. |
| // to call into more app-specific code (whatever they define) | ||
| Custom(T), | ||
| Staking(StakingMsg), | ||
| Distribution(DistributionMsg), |
There was a problem hiding this comment.
Once upon a time, StakingMsg was feature gated under #[cfg(feature = "staking")]. I think it should be again, and same with DistributionMsg. This means that every contract that uses them is forced to expose the requires_staking export flag and then can fail on upload to chains without PoS (I expect some PoA chains and PoE chains to use CosmWasm without staking and delegation).
There was a problem hiding this comment.
It was introduced in #300 and at some point along the way we decided to remove it at least for messages. I don't member why. Maybe it was related to the exhaustive/non-exhaustive and related trouble on the enum. But since it works well for the stargate flag now, I'll just try to add it again.
|
Yeah, makes sense, will update |
webmaster128
force-pushed
the
split-withdraw
branch
from
22aaa6f to
494cf17
Compare
Closes #848