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

[FEATURE] Add cosmwasm-std message for community pool #288

Closed
3 of 4 tasks
lunc808 opened this issue Jun 21, 2023 · 13 comments
Closed
3 of 4 tasks

[FEATURE] Add cosmwasm-std message for community pool #288

lunc808 opened this issue Jun 21, 2023 · 13 comments
Assignees
Labels
enhancement New feature or request state machine breaking Something that will impact the state machine and consensus
Milestone

Comments

@lunc808
Copy link

lunc808 commented Jun 21, 2023

We are requesting to add the cosmwasm-std crate so we can implement the necessary bindings into our smart contracts.

Edit by @fragwuerdig

When I understand it correctly you want us to add MsgFundCommunityPool to the wasm bindings so you can use it in smart contracts. Which is an excellent idea imho.

  • Evaluate whether this can be added to wasmd interface or inside core as terra custom binding
  • Add wasm binding for MsgFundCommunityPool from the distribution module accordingly
  • Update either CosmWasm::std crate (would need upstream push) or terra-bindings repository accordingly.
  • Write some rust example contract on how to use it
@lunc808 lunc808 added the enhancement New feature or request label Jun 21, 2023
@inon-man
Copy link
Collaborator

inon-man commented Jun 21, 2023

It's compatible with the official wasmvm v1.1.2 and cosmwasm v1.2. However, MsgFundCommunityPool has to be added to wasmvm and cosmwasm

@fragwuerdig
Copy link
Collaborator

@inon-man: I wonder If it would makes sense to add it into the terra-bindings and provide it as custom msg. This way we won't need to push it upstream - which probably takes ages to get merged and available for dApp builders on classic?

@fragwuerdig fragwuerdig self-assigned this Jun 21, 2023
@inon-man
Copy link
Collaborator

@inon-man: I wonder If it would makes sense to add it into the terra-bindings and provide it as custom msg. This way we won't need to push it upstream - which probably takes ages to get merged and available for dApp builders on classic?

I think it is possible and probably the best way to go.

@fragwuerdig
Copy link
Collaborator

@inon-man: I wonder If it would makes sense to add it into the terra-bindings and provide it as custom msg. This way we won't need to push it upstream - which probably takes ages to get merged and available for dApp builders on classic?

I think it is possible and probably the best way to go.

Ok. I will try to push it upstream in parallel.

@webmaster128
Copy link

Hi! Happy to add this upstream. In the meantime also note this Anybuf implementation as a workaround for contract devs:

@fragwuerdig
Copy link
Collaborator

Hey. Just for the sake of completeness I am adding all the PRs for this feature in this list:

We can backport these features into our downstream repos so they can be used by contract owners as git dependency in their corresponding Cargo.toml. @inon-man: Is adding the FundCommunityPoolMsg to wasmd consensus breaking? I feel like it is.

@inon-man
Copy link
Collaborator

inon-man commented Jun 23, 2023

Hey. Just for the sake of completeness I am adding all the PRs for this feature in this list:

We can backport these features into our downstream repos so they can be used by contract owners as git dependency in their corresponding Cargo.toml. @inon-man: Is adding the FundCommunityPoolMsg to wasmd consensus breaking? I feel like it is.

I don't think it would break the consensus? Where do you feel it so from?

@fragwuerdig
Copy link
Collaborator

fragwuerdig commented Jun 23, 2023

@inon-man: I was thinking: A validator that accepts the message from the mempool and deserializes it correctly will pass it on to be further processed. A validator that not matches against the new DistributionMsg variant would fail to process it. So state for both validators would end up being different. Thus I was believing it breaks consensus...

@inon-man
Copy link
Collaborator

inon-man commented Jun 24, 2023

It is just a binding for the msg already supported by Cosmos SDK. Imagine that there was no MsgSend in cosmwasm.

@fragwuerdig I think you are right. It's a state breaking change.

@inon-man inon-man added the state machine breaking Something that will impact the state machine and consensus label Jun 24, 2023
@LuncBurner LuncBurner added this to the Proposal 11462 milestone Jul 5, 2023
@nghuyenthevinh2000
Copy link
Member

@inon-man does this one needs a migration logic in upgrade?

@nghuyenthevinh2000
Copy link
Member

@fragwuerdig do you have an example contract on this?

@fragwuerdig
Copy link
Collaborator

fragwuerdig commented Aug 21, 2023

@nghuyenthevinh2000: I can certainly make one... One that sends its own balance to CP using a permissionless trigger msg. Make an issue and assign me if you like.

@nghuyenthevinh2000
Copy link
Member

@nghuyenthevinh2000: I can certainly make one... One that sends its own balance to CP using a permissionless trigger msg. Make an issue and assign me if you like.

you can find it here: #326

@inon-man inon-man modified the milestones: Proposal 11462, v2.2.0 Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request state machine breaking Something that will impact the state machine and consensus
Projects
Status: Done
Development

No branches or pull requests

6 participants