search

DA0-DA0 / dao-contracts

CosmWasm smart contracts for Interchain DAOs.
https://docs.daodao.zone
BSD 3-Clause "New" or "Revised" License
202 stars 133 forks source link

Bounties Contract #715

Open JakeHartnell opened 1 year ago

JakeHartnell commented 1 year ago

Closes #714

codecov[bot] commented 1 year ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (development@0367c84). Click here to learn what that means. Patch coverage: 88.79% of modified lines in pull request are covered. Report is 5 commits behind head on development.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## development #715 +/- ## ============================================== Coverage ? 93.90% ============================================== Files ? 63 Lines ? 5709 Branches ? 0 ============================================== Hits ? 5361 Misses ? 348 Partials ? 0 ``` | [Files Changed](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0) | Coverage Δ | | |---|---|---| | [contracts/external/cw-bounties/src/contract.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy9jb250cmFjdC5ycw==) | `88.79% <88.79%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 year ago

Cosm-Orc Gas Usage

Contract Op Name Gas Used Old Gas Used Gas Diff File
dao_dao_core Store__Store 6297548 6180873 +1.8877% ci/integration-tests/src/helpers/chain.rs:98
cw_vesting Store__Store 4292883 4237984 +1.2954% ci/integration-tests/src/helpers/chain.rs:98
dao_pre_propose_single Store__Store 4279324 4168525 +2.6580% ci/integration-tests/src/helpers/chain.rs:98
dao_proposal_single Store__Store 6789079 6672533 +1.7467% ci/integration-tests/src/helpers/chain.rs:98
dao_voting_cw721_staked Instantiate__instantiate_dao_voting_cw721_staked 180303 178255 +1.1489% ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:49
dao_voting_cw721_staked Store__Store 4648095 3348121 +38.8270% ci/integration-tests/src/helpers/chain.rs:98
cw20_stake Store__Store 4039409 3958328 +2.0484% ci/integration-tests/src/helpers/chain.rs:98
cw20_stake_external_rewards Store__Store 3389630 3350513 +1.1675% ci/integration-tests/src/helpers/chain.rs:98
cw20_stake_reward_distributor Store__Store 2897606 2841017 +1.9919% ci/integration-tests/src/helpers/chain.rs:98
cw_admin_factory Store__Store 2071755 2019807 +2.5719% ci/integration-tests/src/helpers/chain.rs:98
cw_fund_distributor Store__Store 3433089 3362993 +2.0843% ci/integration-tests/src/helpers/chain.rs:98
cw_payroll_factory Store__Store 3756022 3698198 +1.5636% ci/integration-tests/src/helpers/chain.rs:98
cw_token_swap Store__Store 2429112 2382897 +1.9394% ci/integration-tests/src/helpers/chain.rs:98
dao_migrator Store__Store 5202909 5126651 +1.4875% ci/integration-tests/src/helpers/chain.rs:98
dao_pre_propose_approval_single Store__Store 5040906 4924660 +2.3605% ci/integration-tests/src/helpers/chain.rs:98
dao_pre_propose_approver Store__Store 3765057 3675123 +2.4471% ci/integration-tests/src/helpers/chain.rs:98
dao_pre_propose_multiple Store__Store 4358416 4246824 +2.6277% ci/integration-tests/src/helpers/chain.rs:98
dao_proposal_condorcet Store__Store 4712211 4625956 +1.8646% ci/integration-tests/src/helpers/chain.rs:98
dao_proposal_multiple Store__Store 6611576 6470617 +2.1784% ci/integration-tests/src/helpers/chain.rs:98
dao_voting_cw20_staked Store__Store 3665035 3622525 +1.1735% ci/integration-tests/src/helpers/chain.rs:98
dao_voting_cw4 Store__Store 2665777 2686291 -0.7637% ci/integration-tests/src/helpers/chain.rs:98
dao_voting_native_staked Store__Store 3032182 2989815 +1.4170% ci/integration-tests/src/helpers/chain.rs:98
Raw Report for 19332795c0a0ac81e0ad845faabb4937d12c4dc0
| Contract | Op Name | Gas Used | Gas Wanted | File | | --- | --- | --- | --- | --- | | dao_dao_core | Instantiate__inst_dao_no_admin | 1275506 | 1890450 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Instantiate__inst_admin_create_dao | 1276556 | 1892027 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Execute__exc_items_rm | 192822 | 266513 | ci/integration-tests/src/tests/cw_core_test.rs:171 | | dao_dao_core | Execute__exc_items_set | 194489 | 269016 | ci/integration-tests/src/tests/cw_core_test.rs:136 | | dao_dao_core | Instantiate__exc_items_create_dao | 1276556 | 1892027 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Instantiate__create_dao | 1275506 | 1890450 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Instantiate__exc_stake_create_dao | 1275482 | 1890414 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Store__Store | 6297548 | 9423533 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_dao_core | Execute__exc_admin_msgs_pause_dao | 194615 | 269205 | ci/integration-tests/src/tests/cw_core_test.rs:76 | | dao_dao_core | Instantiate__exc_admin_msgs_create_dao | 1275506 | 1890450 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_dao_core | Instantiate__exc_admin_msgs_create_dao_with_admin | 1276556 | 1892027 | ci/integration-tests/src/helpers/helper.rs:101 | | cw_vesting | Execute__delegate | 206783 | 287457 | ci/integration-tests/src/tests/cw_vesting_test.rs:103 | | cw_vesting | Execute__undelegate | 251879 | 332456 | ci/integration-tests/src/tests/cw_vesting_test.rs:142 | | cw_vesting | Execute__withdraw_reward | 191369 | 264333 | ci/integration-tests/src/tests/cw_vesting_test.rs:124 | | cw_vesting | Instantiate__instantiate | 227550 | 318608 | ci/integration-tests/src/tests/cw_vesting_test.rs:59 | | cw_vesting | Store__Store | 4292883 | 6416535 | ci/integration-tests/src/helpers/chain.rs:98 | | cw20_base | Execute__cw20_base_increase_allowance | 142201 | 190586 | ci/integration-tests/src/helpers/helper.rs:216 | | cw20_base | Execute__send_and_stake_cw20 | 234268 | 328683 | ci/integration-tests/src/helpers/helper.rs:180 | | cw20_base | Execute__exc_stake_stake_tokens | 234536 | 320954 | ci/integration-tests/src/tests/cw20_stake_test.rs:76 | | cw20_base | Store__Store | 4191041 | 6263772 | ci/integration-tests/src/helpers/chain.rs:98 | | cw721_base | Instantiate__instantiate_cw721_base | 166518 | 227061 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:22 | | cw721_base | Store__Store | 3975657 | 5940696 | ci/integration-tests/src/helpers/chain.rs:98 | | cw721_base | Execute__mint_nft | 140715 | 188355 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:96 | | cw721_base | Execute__stake_nft | 234592 | 329169 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:79 | | dao_pre_propose_single | Execute__pre_propose_propose | 1806427 | 2686829 | ci/integration-tests/src/helpers/helper.rs:235 | | dao_pre_propose_single | Store__Store | 4279324 | 6396197 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_single | Execute__dao_proposal_single_vote | 5992028 | 8965158 | ci/integration-tests/src/helpers/helper.rs:285 | | dao_proposal_single | Store__Store | 6789079 | 10160769 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw721_staked | Execute__claim_nfts | 5710872 | 8543412 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:129 | | dao_voting_cw721_staked | Instantiate__instantiate_dao_voting_cw721_staked | 180303 | 247739 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:49 | | dao_voting_cw721_staked | Store__Store | 4648095 | 6949353 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw721_staked | Execute__unstake_nfts | 230320 | 322761 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:114 | | multiple_contracts | Execute__batch_cw721_stake_max_claims | 4198067 | 6256677 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:258 | | cw20_stake | Store__Store | 4039409 | 6036324 | ci/integration-tests/src/helpers/chain.rs:98 | | cw20_stake_external_rewards | Store__Store | 3389630 | 5061656 | ci/integration-tests/src/helpers/chain.rs:98 | | cw20_stake_reward_distributor | Store__Store | 2897606 | 4323620 | ci/integration-tests/src/helpers/chain.rs:98 | | cw4_group | Store__Store | 2793580 | 4167581 | ci/integration-tests/src/helpers/chain.rs:98 | | cw721_roles | Store__Store | 5063861 | 7573002 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_admin_factory | Store__Store | 2071755 | 3084843 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_bounties | Store__Store | 2849220 | 4251041 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_fund_distributor | Store__Store | 3433089 | 5126844 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_payroll_factory | Store__Store | 3756022 | 5611244 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_token_swap | Store__Store | 2429112 | 3620879 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_migrator | Store__Store | 5202909 | 7781574 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_approval_single | Store__Store | 5040906 | 7538565 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_approver | Store__Store | 3765057 | 5624796 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_multiple | Store__Store | 4358416 | 6514835 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_condorcet | Store__Store | 4712211 | 7045527 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_multiple | Store__Store | 6611576 | 9894575 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw20_staked | Store__Store | 3665035 | 5474763 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw4 | Store__Store | 2665777 | 3975876 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw721_roles | Store__Store | 2839938 | 4237118 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_native_staked | Store__Store | 3032182 | 4525484 | ci/integration-tests/src/helpers/chain.rs:98 |
taitruong commented 11 months ago

@JakeHartnell, regarding ExecuteMsg::PayOut: https://github.com/DA0-DA0/dao-contracts/blob/bounties/contracts/external/cw-bounties/src/msg.rs#L30

    /// Claims a bounty (only owner)
    PayOut {
        /// Bounty id to claim
        id: u64,
        /// Recipient address where funds from bounty are claimed
        recipient: String,
    },

We could add amount: Option<Uint128> for allowing partial payout - if none is provided then it is a full payout. This way it allows use cases like:

taitruong commented 11 months ago

For Close and PayOut maybe there should be an optional memo. So owner can add reason or other notes.

taitruong commented 11 months ago

@JakeHartnell, regarding ExecuteMsg::PayOut: https://github.com/DA0-DA0/dao-contracts/blob/bounties/contracts/external/cw-bounties/src/msg.rs#L30

    /// Claims a bounty (only owner)
    PayOut {
        /// Bounty id to claim
        id: u64,
        /// Recipient address where funds from bounty are claimed
        recipient: String,
    },

We could add amount: Option<Uint128> for allowing partial payout - if none is provided then it is a full payout. This way it allows use cases like:

  • partial pay out depending on defined/reached milestones
  • awarding multiple peeps contributing to same bounty

Ofc, this is not needed, since owner can create multiple bounties for achieving same goal. But this way, owner can decide which way is appropiate for bounty ((one bounty with partial payout or multiple bounties with each single payout). This change shouldn't be a big deal and keeps contract still simple.

taitruong commented 11 months ago

@JakeHartnell, regarding ExecuteMsg::PayOut: https://github.com/DA0-DA0/dao-contracts/blob/bounties/contracts/external/cw-bounties/src/msg.rs#L30

    /// Claims a bounty (only owner)
    PayOut {
        /// Bounty id to claim
        id: u64,
        /// Recipient address where funds from bounty are claimed
        recipient: String,
    },

We could add amount: Option<Uint128> for allowing partial payout - if none is provided then it is a full payout. This way it allows use cases like:

  • partial pay out depending on defined/reached milestones
  • awarding multiple peeps contributing to same bounty

Nevermind. Figured out partial payment is way too complicated. Like when updating on partial payouts, it will affect BountyStatus when updated amount is equal to partial payout -> Bounty is closed otherwise it keeps open, Bounty need another prop claimed for updated payments made so far. I was almost done implementing this including tests - but trashed it since contract would be too complicated without big value.

taitruong commented 11 months ago

@JakeHartnell, found another bug when updating bounty with lower amount. Like bounty is 100 Juno, update to 50 Juno, then owner gets 50 Juno back. This works. But if owner accidentally sends 50 Juno, 100 Juno should be send back - not just 50.

Fix it in PR 730. Also added more tests extensively and checking Bounty state on each test.

taitruong commented 11 months ago

@JakeHartnell, found another bug when updating bounty with lower amount. Like bounty is 100 Juno, update to 50 Juno, then owner gets 50 Juno back. This works. But if owner accidentally sends 50 Juno, 100 Juno should be send back - not just 50.

Fix it in PR 730. Also added more tests extensively and checking Bounty state on each test.

Check here: https://github.com/DA0-DA0/dao-contracts/pull/730

codecov[bot] commented 8 months ago

Codecov Report

Attention: 50 lines in your changes are missing coverage. Please review.

Comparison is base (db42d45) 96.25% compared to head (5f90c8b) 96.21%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## development #715 +/- ## =============================================== - Coverage 96.25% 96.21% -0.04% =============================================== Files 203 208 +5 Lines 50082 50913 +831 =============================================== + Hits 48207 48988 +781 - Misses 1875 1925 +50 ``` | [Files](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0) | Coverage Δ | | |---|---|---| | [contracts/external/cw-bounties/src/state.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy9zdGF0ZS5ycw==) | `100.00% <100.00%> (ø)` | | | [contracts/external/cw-bounties/src/error.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy9lcnJvci5ycw==) | `50.00% <50.00%> (ø)` | | | [contracts/external/cw-bounties/src/msg.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy9tc2cucnM=) | `50.00% <50.00%> (ø)` | | | [contracts/external/cw-bounties/src/tests.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy90ZXN0cy5ycw==) | `96.52% <96.52%> (ø)` | | | [contracts/external/cw-bounties/src/contract.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL2V4dGVybmFsL2N3LWJvdW50aWVzL3NyYy9jb250cmFjdC5ycw==) | `88.42% <88.42%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JakeHartnell commented 7 months ago

Will merge and ship this after we're done with the veto stuff. :heart:

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 94.26523% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 96.27%. Comparing base (d59c282) to head (793085a).

Files Patch % Lines
contracts/external/cw-bounties/src/contract.rs 89.08% 25 Missing :warning:
contracts/external/cw-bounties/src/tests.rs 96.50% 21 Missing :warning:
contracts/external/cw-bounties/src/error.rs 0.00% 1 Missing :warning:
contracts/external/cw-bounties/src/msg.rs 75.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## development #715 +/- ## =============================================== - Coverage 96.30% 96.27% -0.04% =============================================== Files 207 212 +5 Lines 53490 54327 +837 =============================================== + Hits 51513 52302 +789 - Misses 1977 2025 +48 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.