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

Add make_addr function to MockApi #1905

Merged
merged 13 commits into from
Oct 6, 2023
Merged

Conversation

DariuszDepta
Copy link
Member

@DariuszDepta DariuszDepta commented Oct 5, 2023

closes #1887

@DariuszDepta DariuszDepta self-assigned this Oct 5, 2023
@DariuszDepta DariuszDepta linked an issue Oct 5, 2023 that may be closed by this pull request
@DariuszDepta
Copy link
Member Author

@chipshort @webmaster128

Default settings for Bech32 encoded addresses are:

  • prefix: cosmwasm,
  • variant: Bech32,
  • input digest length: 32 B.

To have an option to change the default settings during MockApi instantiation, I would suggest adding a constructor named with_bech32, that will provide appropriate initialization values, like:

let mock_api = MockApi::with_bech32("juno", false, false);

where the definition could be like:

pub fn with_bech32(prefix: &'static str, bech32m: bool, short: bool) -> MockApi

where the arguments would be:

  • prefix - any reference to static string,
  • bech32m - true => Bech32m variant, false => Bech32 variant
  • short - true => 20 bytes of input digest, false => 32 bytes of input digest

To make function addr_make with Bech32 address encoding more like opt-in, we could set the default prefix to an empty string, then this function would be an equivalent of Addr::unchecked:

let mock_api = MockApi::default();
let addr = mock_api.addr_make("creator");
assert_eq!(Addr::unchecked("creator"), addr);

but initializing MockApi with a non-empty prefix would create an address in Bech32 or Bech32m encoding:

let mock_api = MockApi::with_bech32("juno", false, false);
let addr = mock_api.addr_make("creator");
assert_eq!("juno1h34lmpywh4upnjdg90cjf4j70aee6z8qqfspugamjp42e4q28kqsksmtyp", addr.to_string());

This way we could preserve the old behaviour in existing tests, so users could gradually migrate from using Addr::unchecked to MockApi::addr_make without the need to change any logic or checks in tests.
Finally users could switch the MockApi initialization to generate Bech32 encoded addresses and gradually fix (if needed) existing tests. But this is just an idea to be considered before we complete this implementation.

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Looks good overall. Few points inline

packages/std/Cargo.toml Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
@DariuszDepta DariuszDepta added this to the 1.5.0 milestone Oct 5, 2023
Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Great stuff. Just a few smaller things.

I'd appreciate if @chipshort could do another review and wrap this up for 1.5. We should have a CHANGELOG entry here.

packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
packages/std/src/testing/mock.rs Show resolved Hide resolved
Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

Looks good, just one small comment.

packages/std/src/testing/mock.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

LGTM

@DariuszDepta DariuszDepta merged commit e538daf into main Oct 6, 2023
29 checks passed
@DariuszDepta DariuszDepta deleted the 1887-add-mockapi-addr-make branch October 6, 2023 13:00
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.

Add MockApi::addr_make
3 participants