Skip to content
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.

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

Some multitest bindings for staking are missing such as BondedDenom #753

Closed
shanev opened this issue Jul 16, 2022 · 5 comments · Fixed by #782
Closed

Some multitest bindings for staking are missing such as BondedDenom #753

shanev opened this issue Jul 16, 2022 · 5 comments · Fixed by #782
Milestone

Comments

@shanev
Copy link
Contributor

shanev commented Jul 16, 2022

This causes deps.querier.query_bonded_denom() to fail in mulitest.

@ethanfrey ethanfrey added this to the v0.15.0 milestone Aug 15, 2022
@ethanfrey
Copy link
Member

@shanev can you provide more details here, like a quick code sample to demo?

It can either be:

  1. Building fully functional mocks for Staking (which is useful, but I would push to 0.16 not this release)
  2. Export a few more missing types from multitest to allow a 3rd party to implement it (I would do this right away)

I know there was some other bug that someone couldn't build a multi-test mock for Distribution due to missing exports (it wasn't pub). We should check and address all those, maybe trying to build sample/failing mocks there in another module to show we have all types.

@ueco-jb
Copy link
Member

ueco-jb commented Sep 13, 2022

Building fully functional mocks for Staking (which is useful, but I would push to 0.16 not this release)

Some time ago I started tinkering around that.
https://github.com/CosmWasm/cw-plus/pull/782/files
It's kind of tricky to properly utilize tokens from Bank module storage, "temporary" I created own STAKING storage which acts as staked balances.

@ethanfrey
Copy link
Member

Okay, then we just verify all types are exported. And close this, leaving some "future issue" for proper multi-test staking support

@maurolacy maurolacy self-assigned this Sep 14, 2022
@maurolacy
Copy link
Contributor

maurolacy commented Sep 14, 2022

Okay, then we just verify all types are exported. And close this, leaving some "future issue" for proper multi-test staking support

Regarding this, not exactly sure what needs to be done.

BondedDenom is published as part of StakingQuery in cosmwasm-std

https://github.com/CosmWasm/cosmwasm/blob/421f75cde11ff8842fd279ea3ec2cbcca65ef399/packages/std/src/query/staking.rs#L11-L34

along with the other staking queries.

The same with the staking messages

https://github.com/CosmWasm/cosmwasm/blob/421f75cde11ff8842fd279ea3ec2cbcca65ef399/packages/std/src/results/cosmos_msg.rs#L76-L90

What's required is implementing the Module trait for staking in multi-test, that is, adding support for handling the staking messages and queries. But that's out of scope here.

So, unless I'm missing something related to exporting these types, I would say, the exporting / publishing part is already done.

@maurolacy maurolacy modified the milestones: 0.15.0, 0.16.0 Sep 14, 2022
@ethanfrey
Copy link
Member

@ueco-jb has started work on better testing in #782

I assume this is what is desired (when finished)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants