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

Generalized querier getter #318

Merged

Conversation

CyberHoward
Copy link
Contributor

This draft PR shows a different approach to querier construction and access.

Instead of having a Getter trait for each querier we have a single QuerierGetter trait that retrieves a querier Q. This querier Q implements a Querier trait. Both are shown below:

pub trait QuerierGetter<Q: Querier> {
    fn querier(&self) -> Q;
}

pub trait Querier {
    type Error: Into<CwEnvError> + Debug;
}

The inclusion of type Error in Querier prevents some duplicate code in the traits.

Now each querier trait needs to add Querier as its trait bound, like so:

pub trait BankQuerier: Querier {
    fn supply_of(&self, denom: impl Into<String>) -> Result<Coin, Self::Error>;
    // ...
}

To get allow for retrieving the querier we implement the QuerierGetter with a specified querier like so:

impl Querier for DaemonBankQuerier {
    type Error = DaemonError;
}

impl QuerierGetter<DaemonBankQuerier> for Daemon {
    fn querier(&self) -> DaemonBankQuerier {
        DaemonBankQuerier::new(self)
    }
}

We can then create arbitrary sets of supported queriers, like:

pub trait DefaultQueriers: QuerierGetter<Self::B> + QuerierGetter<Self::W> + QuerierGetter<Self::N> {
    type B: BankQuerier;
    type W: WasmQuerier;
    type N: NodeQuerier;

    fn bank_querier(&self) -> Self::B {
        self.querier()
    }

    fn wasm_querier(&self) -> Self::W {
        self.querier()
    }

    fn node_querier(&self) -> Self::N {
        self.querier()
    }
}

And use that as a trait-bound!

pub trait QueryHandler: DefaultQueriers {
  // ...
}

Thoughts?

Copy link

cloudflare-workers-and-pages bot commented Jan 23, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2b78247
Status:🚫  Build failed.

View logs

@CyberHoward
Copy link
Contributor Author

@Kayanski do you prefer this over your original design? If so I can proceed with finalizing this.

@Kayanski
Copy link
Contributor

Kayanski commented Jan 24, 2024

@Kayanski do you prefer this over your original design? If so I can proceed with finalizing this.

yes this is a good idea !

@CyberHoward CyberHoward marked this pull request as ready for review January 29, 2024 14:32
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 266 lines in your changes are missing coverage. Please review.

Comparison is base (a990cbe) 66.2% compared to head (2b78247) 65.5%.
Report is 9 commits behind head on update/separate-query-tx.

Additional details and impacted files
Files Coverage Δ
cw-orch-daemon/src/sync/core.rs 52.5% <100.0%> (-4.4%) ⬇️
...ges/cw-orch-core/src/contract/contract_instance.rs 78.4% <ø> (ø)
...ages/cw-orch-core/src/contract/interface_traits.rs 73.9% <100.0%> (+3.3%) ⬆️
...-orch-core/src/environment/cosmwasm_environment.rs 30.1% <100.0%> (+11.3%) ⬆️
...kages/cw-orch-core/src/environment/queriers/mod.rs 100.0% <100.0%> (ø)
...ages/cw-orch-core/src/environment/queriers/wasm.rs 100.0% <100.0%> (ø)
packages/cw-orch-core/src/environment/state.rs 55.5% <ø> (ø)
packages/cw-orch-mock/src/core.rs 93.3% <100.0%> (+5.8%) ⬆️
...kages/cw-orch-core/src/environment/queriers/env.rs 0.0% <0.0%> (ø)
packages/cw-orch-core/src/environment/mut_env.rs 0.0% <0.0%> (ø)
... and 16 more

... and 1 file with indirect coverage changes

@Kayanski Kayanski changed the base branch from update/separate-query-tx to main January 29, 2024 16:06
@Kayanski Kayanski changed the base branch from main to update/separate-query-tx January 29, 2024 16:06
@@ -25,7 +25,7 @@ eth = ["dep:snailquote"]

[dependencies]
thiserror = { workspace = true }
cosmwasm-std = { workspace = true }
cosmwasm-std.workspace = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change, is it linting or manual ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was giving me issues for some reason, don't know why

@CyberHoward CyberHoward changed the title POC of generalized querier getter Generalized querier getter Jan 30, 2024
Copy link
Contributor

@Kayanski Kayanski left a comment

Choose a reason for hiding this comment

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

LGTM

@CyberHoward CyberHoward merged commit 1e0c97f into update/separate-query-tx Jan 31, 2024
16 of 18 checks passed
@CyberHoward CyberHoward deleted the update/seperate-query-tx-unify-getter branch January 31, 2024 09:55
Kayanski added a commit that referenced this pull request Feb 5, 2024
* Added first queriers

* Added bank + Wasm traits

* Removed deprecated code

* Lint

* Format

* Generalized querier getter (#318)

* show POC of generalized querier getter

* update

* fix merge and impl for all envs

* Doon't verify mockj error anymore

* Fixed test

* update `QueryHandler` and WasmQuerier functions

* fix smart query URL

* fix compile

* format and clippy

---------

Co-authored-by: Kayanski <kowalski.kowalskin@gmail.com>

* Added instantiate2 (#309)

* Added instantiate2

* some fixes

* Added instantiate2 addr getter

* Format

* Nits

* Merged daemon queriers

* Corrected querier in tests

* Renamed queriers

* fix tests

* Fix tests

* Renamed trait types

* Changed types for osmosis test tube

* Feature fix

* Removed unused querier traits, adapted all daemon queriers with same structure

---------

Co-authored-by: CyberHoward <88450409+CyberHoward@users.noreply.github.com>
Kayanski added a commit that referenced this pull request Feb 5, 2024
* Added first queriers

* Added bank + Wasm traits

* Added cw-multi-test 0.20.0

* Removed secp, removed ibc-relayer-types

* Removed ibc-relayer-types completely

* updated prost typesé

* Start testing authz

* Added authz test

* Removed deprecated code

* Lint

* Format

* add authz querier

* tx handler error with anyhow test

* format

* Added authz builder method for options

* Some nits

* Revert with_authz, added sender options setter

* Check rest of authz queries

* Added mock state env id

* Added fee grant in daemon builder and interface

* Format

* fix find_wasm_path_with

* Generalized querier getter (#318)

* show POC of generalized querier getter

* update

* fix merge and impl for all envs

* Doon't verify mockj error anymore

* Fixed test

* update `QueryHandler` and WasmQuerier functions

* fix smart query URL

* fix compile

* format and clippy

---------

Co-authored-by: Kayanski <kowalski.kowalskin@gmail.com>

* Added instantiate2 (#309)

* Added instantiate2

* some fixes

* Added instantiate2 addr getter

* Format

* Nits

* Merged daemon queriers

* Corrected querier in tests

* Renamed queriers

* fix tests

* Fix tests

* Renamed trait types

* Changed types for osmosis test tube

* Fixed features

---------

Co-authored-by: Buckram <buckram123@gmail.com>
Co-authored-by: CyberHoward <88450409+CyberHoward@users.noreply.github.com>
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 this pull request may close these issues.

2 participants