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

Dao voting cw4 cleanup #691

Closed JakeHartnell closed 1 year ago

JakeHartnell commented 1 year ago

Closes #675

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 96.00% and project coverage change: +0.12 :tada:

Comparison is base (9261bb3) 93.82% compared to head (ae9600a) 93.94%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #691 +/- ## ========================================== + Coverage 93.82% 93.94% +0.12% ========================================== Files 60 60 Lines 5149 5189 +40 ========================================== + Hits 4831 4875 +44 + Misses 318 314 -4 ``` | [Impacted Files](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/691?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0) | Coverage Δ | | |---|---|---| | [contracts/voting/dao-voting-cw4/src/contract.rs](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/691?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0#diff-Y29udHJhY3RzL3ZvdGluZy9kYW8tdm90aW5nLWN3NC9zcmMvY29udHJhY3QucnM=) | `94.18% <96.00%> (-1.31%)` | :arrow_down: | ... and [21 files with indirect coverage changes](https://app.codecov.io/gh/DA0-DA0/dao-contracts/pull/691/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DA0-DA0)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

github-actions[bot] commented 1 year ago

Cosm-Orc Gas Usage

Contract Op Name Gas Used Old Gas Used Gas Diff File
cw721_base Instantiate__instantiate_cw721_base 166518 164837 +1.0198% ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:22
cw721_base Store__Store 3975657 3369402 +17.9930% ci/integration-tests/src/helpers/chain.rs:98
dao_voting_cw4 Store__Store 2613465 2684822 -2.6578% ci/integration-tests/src/helpers/chain.rs:98
Raw Report for ae9600a39a83da3af47cfb31fc675717fcffb011
| Contract | Op Name | Gas Used | Gas Wanted | File | | --- | --- | --- | --- | --- | | dao_core | Instantiate__inst_admin_create_dao | 1276299 | 1891644 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Instantiate__exc_stake_create_dao | 1275225 | 1890033 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Execute__exc_admin_msgs_pause_dao | 194613 | 269202 | ci/integration-tests/src/tests/cw_core_test.rs:76 | | dao_core | Instantiate__exc_admin_msgs_create_dao | 1275249 | 1890069 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Instantiate__exc_admin_msgs_create_dao_with_admin | 1276299 | 1891644 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Execute__exc_items_rm | 192820 | 266513 | ci/integration-tests/src/tests/cw_core_test.rs:171 | | dao_core | Execute__exc_items_set | 194487 | 269013 | ci/integration-tests/src/tests/cw_core_test.rs:136 | | dao_core | Instantiate__exc_items_create_dao | 1276299 | 1891644 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Instantiate__create_dao | 1275249 | 1890069 | ci/integration-tests/src/helpers/helper.rs:101 | | dao_core | Store__Store | 6177688 | 9243743 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_core | Instantiate__inst_dao_no_admin | 1275249 | 1890069 | ci/integration-tests/src/helpers/helper.rs:101 | | cw20_base | Execute__exc_stake_stake_tokens | 234536 | 320952 | ci/integration-tests/src/tests/cw20_stake_test.rs:76 | | cw20_base | Execute__cw20_base_increase_allowance | 142201 | 190586 | ci/integration-tests/src/helpers/helper.rs:216 | | cw20_base | Execute__send_and_stake_cw20 | 234267 | 328683 | ci/integration-tests/src/helpers/helper.rs:180 | | cw20_base | Store__Store | 4180849 | 6248484 | 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 | Execute__mint_nft | 140715 | 188355 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:93 | | cw721_base | Execute__stake_nft | 234591 | 329169 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:76 | | cw721_base | Store__Store | 3975657 | 5940696 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_single | Execute__pre_propose_propose | 1806420 | 2686818 | ci/integration-tests/src/helpers/helper.rs:235 | | dao_pre_propose_single | Store__Store | 4168135 | 6229413 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_single | Execute__dao_proposal_single_vote | 5992046 | 8965184 | ci/integration-tests/src/helpers/helper.rs:285 | | dao_proposal_single | Store__Store | 6674821 | 9989442 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_vesting | Execute__delegate | 206743 | 287399 | ci/integration-tests/src/tests/cw_vesting_test.rs:103 | | cw_vesting | Execute__undelegate | 251835 | 332390 | ci/integration-tests/src/tests/cw_vesting_test.rs:142 | | cw_vesting | Execute__withdraw_reward | 191362 | 264323 | ci/integration-tests/src/tests/cw_vesting_test.rs:124 | | cw_vesting | Instantiate__instantiate | 227547 | 318603 | ci/integration-tests/src/tests/cw_vesting_test.rs:59 | | cw_vesting | Store__Store | 4236619 | 6332139 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw721_staked | Execute__unstake_nfts | 230319 | 322761 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:111 | | dao_voting_cw721_staked | Instantiate__instantiate_dao_voting_cw721_staked | 178857 | 245570 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:49 | | dao_voting_cw721_staked | Store__Store | 3346379 | 4996779 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw721_staked | Execute__claim_nfts | 5710869 | 8543408 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:126 | | cw20_stake | Store__Store | 3956612 | 5912129 | ci/integration-tests/src/helpers/chain.rs:98 | | cw20_stake_external_rewards | Store__Store | 3350045 | 5002278 | ci/integration-tests/src/helpers/chain.rs:98 | | cw20_stake_reward_distributor | Store__Store | 2841017 | 4238736 | ci/integration-tests/src/helpers/chain.rs:98 | | cw4_group | Store__Store | 2783570 | 4152566 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_admin_factory | Store__Store | 2019807 | 3006921 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_fund_distributor | Store__Store | 3365619 | 5025639 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_payroll_factory | Store__Store | 3696547 | 5522031 | ci/integration-tests/src/helpers/chain.rs:98 | | cw_token_swap | Store__Store | 2382910 | 3551576 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_migrator | Store__Store | 5126339 | 7666719 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_approval_single | Store__Store | 4916480 | 7351931 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_approver | Store__Store | 3674720 | 5489291 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_pre_propose_multiple | Store__Store | 4246655 | 6347193 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_condorcet | Store__Store | 4635150 | 6929931 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_proposal_multiple | Store__Store | 6472801 | 9686412 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw20_staked | Store__Store | 3626321 | 5416692 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_cw4 | Store__Store | 2613465 | 3897408 | ci/integration-tests/src/helpers/chain.rs:98 | | dao_voting_native_staked | Store__Store | 2988099 | 4459359 | ci/integration-tests/src/helpers/chain.rs:98 | | multiple_contracts | Execute__batch_cw721_stake_max_claims | 4198001 | 6256577 | ci/integration-tests/src/tests/dao_voting_cw721_staked_test.rs:257 |
JakeHartnell commented 1 year ago

What about admin of existing cw4-group? When creating new group you set it to be the DAO, maybe this contract should verify that the DAO is the admin or its not set?

Good question! IMO this should not be enforced. What about things like SubDAOs where a certain groups contract may be used across multiple SubDAOs?

That said, folks should exercise caution when using existing contracts if they don't understand who controls them. Feel this should be more of a UX thing.

JakeHartnell commented 1 year ago

Made some clean up tickets: