Skip to content

Conversation

@Kayanski
Copy link
Contributor

@Kayanski Kayanski commented Feb 8, 2024

This PR aims at introducing MockBech32 that enforces valid addresses
Tests needs a little revamp (doc tests mostly), but that's a good looking implementation and usable by users.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 8, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1d8c7e2
Status: ✅  Deploy successful!
Preview URL: https://e0853a05.cw-orchestrator.pages.dev
Branch Preview URL: https://fix-mock-bech32.cw-orchestrator.pages.dev

View logs

@codecov
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

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

Comparison is base (3a79f71) 65.4% compared to head (1d8c7e2) 65.4%.

Additional details and impacted files
Files Coverage Δ
cw-orch/src/osmosis_test_tube/core.rs 55.2% <ø> (+0.2%) ⬆️
...ages/cw-orch-core/src/contract/interface_traits.rs 75.9% <100.0%> (ø)
...ages/cw-orch-core/src/environment/queriers/wasm.rs 100.0% <ø> (ø)
packages/cw-orch-core/src/error.rs 57.1% <ø> (ø)
packages/cw-orch-mock/src/core.rs 97.1% <100.0%> (+3.3%) ⬆️
packages/cw-orch-mock/src/queriers/bank.rs 92.8% <100.0%> (+3.2%) ⬆️
packages/cw-orch-mock/src/queriers/env.rs 0.0% <ø> (ø)
packages/cw-orch-mock/src/queriers/mod.rs 0.0% <ø> (ø)
cw-orch-daemon/src/queriers/cosmwasm.rs 47.7% <0.0%> (ø)
cw-orch/src/osmosis_test_tube/queriers/wasm.rs 52.9% <0.0%> (ø)
... and 4 more

... and 7 files with indirect coverage changes

@CyberHoward
Copy link
Contributor

Did a bit of a refactor of the queries to make them API generic by relying on the difference in addr generation between the mock and bech32 impls.

I'm a bit confused by the bech32 module in core.rs. Can we split these two type impls?

@Kayanski
Copy link
Contributor Author

Kayanski commented Feb 9, 2024

Did a bit of a refactor of the queries to make them API generic by relying on the difference in addr generation between the mock and bech32 impls.

I'm a bit confused by the bech32 module in core.rs. Can we split these two type impls?

Haha gotta love the refactor hack :)

--> Yes we can split the 2 impls in separate files, sure ! The differences are very subtle :)

@CyberHoward
Copy link
Contributor

Another question we should ask ourselves is if we want to keep supporting impl Into<String> in our address fields?

@Kayanski
Copy link
Contributor Author

Another question we should ask ourselves is if we want to keep supporting impl Into<String> in our address fields?

See answer above? I think that it's really good to simplify the users life in case no bech32 verification is needed

@CyberHoward CyberHoward merged commit 61369ea into main Feb 13, 2024
@CyberHoward CyberHoward deleted the fix/mock-bech32 branch February 13, 2024 18:18
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.

4 participants