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

Sunny advices not to use this. Are these not properly supported? #39

Open
MbBrainz opened this issue Oct 3, 2022 · 8 comments
Open

Comments

@MbBrainz
Copy link

MbBrainz commented Oct 3, 2022

@sunnya97 Should these bindings be used at all?

@triccs
Copy link

triccs commented Oct 3, 2022

following

@sunnya97
Copy link

sunnya97 commented Oct 3, 2022

@iboss-ptk

@iboss-ptk
Copy link

Our plan is to move away from bindings and use osmois-std instead. The reason is that with osmosis-std uses stargate msgs and stargate queries so that every protobuf services are avaiiable out of the box, the library just provide convenient stargate msg construction. No additional chain code needed for any new bindings.

For queries, they are only available as whitelisted ones here to avoid non-determinism.

@triccs
Copy link

triccs commented Oct 5, 2022

Does this mean bindings won't work in the future? Or just that they won't be expanded?

Because currently bindings allow better functionality for mints and burns but I can't use both (bindings & std) in a single contract due to the typed Response.

@iboss-ptk
Copy link

iboss-ptk commented Oct 6, 2022

@triccs it won't be expanded, but could you elaborate on 2 points:

Because currently bindings allow better functionality for mints and burns

since I'm maintaining osmosis-std would love to see what makes the exp better, could be low hanging fruit.

I can't use both (bindings & std) in a single contract due to the typed Response.

I think for this point it should not be a problem, could you provide specific example so that I can see if I can help with the issue?

@triccs
Copy link

triccs commented Oct 6, 2022

@iboss-ptk
Thank you

Mints don't allow you to specify a denom or a mint_to address. The address is no biggy since you can add a separate send msg but the denom choice makes it so you can't have multiple assets under one admin (i'm assuming since there isn't a way to differentiate).

image
The Response object has to be Response<OsmosisMsg> to be able to send the binding's OsmosisMsgs but it also makes it so you can't also send CosmosMsgs as you'll get an Error.
image

@iboss-ptk
Copy link

iboss-ptk commented Oct 7, 2022

For the first point, it does allow denom choice, since amount actually has type Coin (see MsgMint). So the following code should work.

let msg_mint = MsgMint {
  sender: "<sender_addr>"
  // the `super::super::super::cosmos::base::v1beta1::Coin` type has `impl From<cosmwasm_std::Coin>`
  amount: Some(cosmwasm_std::Coin::new(1_000, "factory/<creator_addr>/<subdenom>").into())
}

For second point, it's actually possible as is but a bit convoluted, this commit should fix it but you can also roll your own. I will get this published soon. Using the main branch will do the trick as well.

@triccs
Copy link

triccs commented Oct 7, 2022

My fault on the first question, that's so simple thank you.

Re: 2nd. Perfect, I'll end up testing this as we expand the proxy contract.

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

No branches or pull requests

4 participants