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 cw20 support to token-weighted group #143

Closed
7 of 10 tasks
ethanfrey opened this issue Nov 13, 2020 · 1 comment · Fixed by #282
Closed
7 of 10 tasks

Add cw20 support to token-weighted group #143

ethanfrey opened this issue Nov 13, 2020 · 1 comment · Fixed by #282
Assignees
Labels
multisig Related to a multisig epic
Milestone

Comments

@ethanfrey
Copy link
Member

ethanfrey commented Nov 13, 2020

Builds on #142

The previous issue only supported native tokens. Extend this to support either one native denom or one cw20 address - set in init. Try for yourself which is the best code reuse - if it is easier to make 2 different contracts, one native-only and one cw20-only and share some group logic between them, that is also fine. Any given contract instance should only take either native coins or cw20 coins, so they don't need both code paths in the contract - think about making this maintainable and bug-free.


Here is a breakdown of the steps needed:

  • Use cw20::Balance instead of String for InitMsg::denom and Config::denom
  • Change handle_bond to accept native of cw20 balance. That means accepting sender: HumanAddr, amount: cw20::Balance instead of info: MessageInfo. This conversion should be done in the handle switch just like in cw20-escrow
  • handle_bond should enforce incoming tokens using a switch on config.denom, inspired by cw20-escrow checks. You will want to keep the current logic for the Balance::Native case (or simplify it to use cw0::must_pay if you know how to extend the custom error type.
  • Update handle_claim to release either native token (as now) or cw20-token (this can be unimplemented!() now, but have the branch based on the cw20::Balance variant)
  • Ensure all current tests pass with minor changes (just wrapping the denom in cw20::Balance::Native. You have laid the groundwork, but still have no cw20 support.
  • Add a HandleMsg::Receive variant to receive cw20 tokens. It should look similar to cw20-escrow::ReceiveMsg, but the ReceiveMsg should only handle the bond case, like pub enum HandleMsg { Bond {} }. The Receive interface is explained in the cw20 spec
  • You need to update handle to process this Receive variant. If it is properly formatted, it should call into handle_bond using a cw20::Balance::Cw20 token rather than cw20::Balance::Native. You can refer to the cw20-escrow receive switch.
  • You can test instantiating a contract with cw20 support and properly bonding. For this, just mock out the receive call. (You can also try cw-multi-test to test a multi-contract setup, but that is still very undocumented. Use at you own risk.
  • Now, go back to handle_claim and ensure it emits the proper withdraw message when the config.denom is cw20::Balance::Cw20.
  • Add end-to-end tests for a cw20-supporting contract: bonding, unbonding, claiming. If you can query the membership value in the bonded group, there is no need to test voting or anything else, just that the membership is properly updated.
@ethanfrey ethanfrey added the multisig Related to a multisig epic label Nov 13, 2020
@maurolacy maurolacy self-assigned this Jan 4, 2021
@juggernaut09
Copy link
Contributor

Working on it.

@ethanfrey ethanfrey added this to the 0.6.1 milestone Mar 16, 2021
@ethanfrey ethanfrey modified the milestones: 0.6.1, 0.6.0 Apr 14, 2021
@ethanfrey ethanfrey self-assigned this Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multisig Related to a multisig epic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants