Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Revert "Remove all use of mint tokens" #25

Merged
merged 8 commits into from
May 10, 2022
Merged

Conversation

maurolacy
Copy link
Collaborator

This reverts commit c0ea5b7.

This re-instates the mint / full-denom mintings.

Copy link
Member

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm

packages/bindings/src/msg.rs Outdated Show resolved Hide resolved
packages/bindings-test/src/multitest.rs Outdated Show resolved Hide resolved
@maurolacy maurolacy force-pushed the enable-mint-bindings branch from cce1047 to 5030d71 Compare May 8, 2022 08:28
Rename EstimatePriceResponse to SwapResponse for consistency / clarity
Copy link
Collaborator

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Good stuff.
I haven't looked at the osmosis side yet to make sure it is compatible, or if it requires another step.
But approve this for the minimal minting functionality

fn build_denom(&self, contract: &Addr, sub_denom: &str) -> String {
// TODO: validation assertion on the full denom.
// https://github.com/cosmos/cosmos-sdk/blob/2646b474c7beb0c93d4fafd395ef345f41afc251/types/coin.go#L706-L711
// Plus, the address must not contain the separator ('/') string.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would minimally assert some limits... len >= 2, len <= 127, no /

packages/bindings/src/msg.rs Show resolved Hide resolved
@maurolacy maurolacy merged commit 2b78651 into main May 10, 2022
@maurolacy maurolacy deleted the enable-mint-bindings branch May 10, 2022 08:11
mergify bot pushed a commit to osmosis-labs/osmosis that referenced this pull request May 16, 2022
Closes / Related to: #1029

## What is the purpose of the change

Re-instate the CoswmWasm message handlers for Mint and FullDenom. Related to osmosis-labs/bindings#25 on the Rust side.


## Brief change log

  - Uncomment the Mint-related code.
  - Implemented `Mint` using `TokenFactory`.
  - Implemented `FullDenom` in terms of `TokenFactory`.

## Testing and Verifying

Uncommented and adapted already existing tests.


## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? (yes. For the CosmWasm side)
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (will do)
  - How is the feature or change documented? (not documented)
larry0x pushed a commit to larry0x/wasmd that referenced this pull request Jul 17, 2023
Closes / Related to: CosmWasm#1029

## What is the purpose of the change

Re-instate the CoswmWasm message handlers for Mint and FullDenom. Related to osmosis-labs/bindings#25 on the Rust side.


## Brief change log

  - Uncomment the Mint-related code.
  - Implemented `Mint` using `TokenFactory`.
  - Implemented `FullDenom` in terms of `TokenFactory`.

## Testing and Verifying

Uncommented and adapted already existing tests.


## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? (yes. For the CosmWasm side)
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? (will do)
  - How is the feature or change documented? (not documented)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants