diff --git a/contract-shared-headers/daccustodian_shared.hpp b/contract-shared-headers/daccustodian_shared.hpp index 9293c6be..fe59c56b 100644 --- a/contract-shared-headers/daccustodian_shared.hpp +++ b/contract-shared-headers/daccustodian_shared.hpp @@ -60,6 +60,11 @@ namespace eosdac { eosio::indexed_by<"bydecayed"_n, eosio::const_mem_fun>, eosio::indexed_by<"byreqpay"_n, eosio::const_mem_fun>>; + using pending_custodians_table = eosio::multi_index<"pendingcusts"_n, custodian, + eosio::indexed_by<"byvotesrank"_n, eosio::const_mem_fun>, + eosio::indexed_by<"bydecayed"_n, eosio::const_mem_fun>, + eosio::indexed_by<"byreqpay"_n, eosio::const_mem_fun>>; + struct [[eosio::table("candidates"), eosio::contract("daccustodian")]] candidate { eosio::name candidate_name; eosio::asset requestedpay; @@ -420,7 +425,7 @@ namespace eosdac { void removeCustodian(name cust, name internal_dac_id); void disableCandidate(name cust, name internal_dac_id); void removeCandidate(name cust, name internal_dac_id); - void allocateCustodians(bool early_election, name internal_dac_id); + void allocateCustodians(name internal_dac_id); bool permissionExists(name account, name permission); bool _check_transaction_authorization(const char *trx_data, uint32_t trx_size, const char *pubkeys_data, uint32_t pubkeys_size, const char *perms_data, uint32_t perms_size); diff --git a/contracts/daccustodian/daccustodian.test.ts b/contracts/daccustodian/daccustodian.test.ts index 8e9d18eb..dcd389bb 100644 --- a/contracts/daccustodian/daccustodian.test.ts +++ b/contracts/daccustodian/daccustodian.test.ts @@ -1614,6 +1614,72 @@ describe('Daccustodian', () => { }); }); + context( + 'First Newperiod with existing custodians after pending change', + async () => { + let dacId = 'pendingdac'; + let regMembers: Account[]; + let newUser1: Account; + let candidates: Account[]; + let number_of_custodians = 5; + before(async () => { + await shared.initDac(dacId, '4,PENDDAC', '1000000.0000 PENDDAC'); + await shared.updateconfig(dacId, '12.0000 PENDDAC'); + await shared.dac_token_contract.stakeconfig( + { enabled: true, min_stake_time: 5, max_stake_time: 20 }, + '4,PENDDAC', + { from: shared.auth_account } + ); + newUser1 = await debugPromise( + AccountManager.createAccount(), + 'create account for capture stake' + ); + + // With 16 voting members with 2000 each and a threshold of 31 percent + // this will total to 320_000 vote value which will be enough to start the DAC + regMembers = await shared.getRegMembers(dacId, '20000.0000 PENDDAC'); + + candidates = await shared.getStakeObservedCandidates( + dacId, + '12.0000 PENDDAC' + ); + + await shared.daccustodian_contract.tstaddcust(candidates[0], dacId); + await shared.daccustodian_contract.tstaddcust(candidates[1], dacId); + await shared.daccustodian_contract.tstaddcust(candidates[2], dacId); + await shared.daccustodian_contract.tstaddcust(candidates[3], dacId); + await shared.daccustodian_contract.tstaddcust(candidates[4], dacId); + + await shared.voteForCustodians(regMembers, candidates, dacId); + }); + it('should succeed with custodians in pendingcusts', async () => { + await shared.daccustodian_contract.newperiod( + 'initial new period', + dacId, + { + from: regMembers[0], // Could be run by anyone. + } + ); + + await assertRowCount( + shared.daccustodian_contract.pendingcustsTable({ + scope: dacId, + limit: 20, + }), + number_of_custodians + ); + }); + it('should leave existing custodians since this is the first election after the change', async () => { + await assertRowCount( + shared.daccustodian_contract.custodians1Table({ + scope: dacId, + limit: 20, + }), + 5 + ); + }); + } + ); context('New Period Elections', async () => { let dacId = 'nperidac'; let regMembers: Account[]; @@ -1684,7 +1750,7 @@ describe('Daccustodian', () => { context( 'without enough candidates with > 0 votes to fill the configs', async () => { - it('should fail with not enough candidates error', async () => { + it('should fail with not enough candidates error 1', async () => { await assertEOSErrorIncludesMessage( shared.daccustodian_contract.newperiod( 'initial new period', @@ -1749,7 +1815,7 @@ describe('Daccustodian', () => { ); chai.expect(actual).to.equal(1); }); - it('should succeed with custodians populated', async () => { + it('should succeed with custodians in pendingcusts', async () => { await shared.daccustodian_contract.newperiod( 'initial new period', dacId, @@ -1759,7 +1825,7 @@ describe('Daccustodian', () => { ); await assertRowCount( - shared.daccustodian_contract.custodians1Table({ + shared.daccustodian_contract.pendingcustsTable({ scope: dacId, limit: 20, }), @@ -1794,7 +1860,7 @@ describe('Daccustodian', () => { keyType: 'i64', }); - let res2 = await shared.daccustodian_contract.custodians1Table({ + let res2 = await shared.daccustodian_contract.pendingcustsTable({ scope: dacId, limit: 100, indexPosition: 2, // bydecayed index @@ -1829,91 +1895,6 @@ describe('Daccustodian', () => { 0 ); }); - it('should set the auths', async () => { - let account = await debugPromise( - EOSManager.rpc.get_account(shared.auth_account.name), - 'get account info' - ); - let permissions = account.permissions.sort( - (a: { perm_name: string }, b: { perm_name: string }) => - a.perm_name.localeCompare(b.perm_name) - ); - - const custodians = - await shared.daccustodian_contract.custodians1Table({ - scope: dacId, - limit: 20, - }); - const expected_accounts = custodians.rows.map((row) => { - return { - permission: { - actor: row.cust_name, - permission: 'active', - }, - weight: 1, - }; - }); - - let ownerPermission = permissions[0]; - let ownerRequiredAuth = ownerPermission.required_auth; - chai.expect(ownerPermission.parent).to.eq('owner'); - chai.expect(ownerPermission.perm_name).to.eq('active'); - chai.expect(ownerRequiredAuth.threshold).to.eq(1); - chai.expect(ownerRequiredAuth.keys.length).to.eq(1); - - let adminPermission = permissions[1]; - let adminRequiredAuth = adminPermission.required_auth; - chai.expect(adminPermission.parent).to.eq('one'); - chai.expect(adminPermission.perm_name).to.eq('admin'); - chai.expect(adminRequiredAuth.threshold).to.eq(1); - chai.expect(adminRequiredAuth.accounts).to.deep.equal([ - { - permission: { - actor: shared.daccustodian_contract.account.name, - permission: 'eosio.code', - }, - weight: 1, - }, - ]); - - let highPermission = permissions[2]; - let highRequiredAuth = highPermission.required_auth; - chai.expect(highPermission.parent).to.eq('active'); - chai.expect(highPermission.perm_name).to.eq('high'); - chai.expect(highRequiredAuth.threshold).to.eq(4); - - let highAccounts = highRequiredAuth.accounts; - chai.expect(highAccounts).to.deep.equal(expected_accounts); - - let lowPermission = permissions[3]; - let lowRequiredAuth = lowPermission.required_auth; - - chai.expect(lowPermission.parent).to.eq('med'); - chai.expect(lowPermission.perm_name).to.eq('low'); - chai.expect(lowRequiredAuth.threshold).to.eq(2); - - let lowAccounts = lowRequiredAuth.accounts; - chai.expect(lowAccounts).to.deep.equal(expected_accounts); - - let medPermission = permissions[4]; - let medRequiredAuth = medPermission.required_auth; - - chai.expect(medPermission.parent).to.eq('high'); - chai.expect(medPermission.perm_name).to.eq('med'); - chai.expect(medRequiredAuth.threshold).to.eq(3); - - let medAccounts = medRequiredAuth.accounts; - chai.expect(medAccounts).to.deep.equal(expected_accounts); - - let onePermission = account.permissions[5]; - let oneRequiredAuth = onePermission.required_auth; - - chai.expect(onePermission.parent).to.eq('low'); - chai.expect(onePermission.perm_name).to.eq('one'); - chai.expect(oneRequiredAuth.threshold).to.eq(1); - let oneAccounts = oneRequiredAuth.accounts; - chai.expect(oneAccounts).to.deep.equal(expected_accounts); - }); }); // it('should succeed setting up testuser', async () => { // await setup_test_user(candidates[0], 'PERDAC'); @@ -2081,6 +2062,97 @@ describe('Daccustodian', () => { } ); }); + it('should set the auths', async () => { + let account = await debugPromise( + EOSManager.rpc.get_account(shared.auth_account.name), + 'get account info' + ); + let permissions = account.permissions.sort( + (a: { perm_name: string }, b: { perm_name: string }) => + a.perm_name.localeCompare(b.perm_name) + ); + + const custodians = + await shared.daccustodian_contract.custodians1Table({ + scope: dacId, + limit: 20, + }); + const expected_accounts = custodians.rows.map((row) => { + return { + permission: { + actor: row.cust_name, + permission: 'active', + }, + weight: 1, + }; + }); + + let ownerPermission = permissions[0]; + let ownerRequiredAuth = ownerPermission.required_auth; + chai.expect(ownerPermission.parent).to.eq('owner'); + chai.expect(ownerPermission.perm_name).to.eq('active'); + chai.expect(ownerRequiredAuth.threshold).to.eq(1); + chai.expect(ownerRequiredAuth.keys.length).to.eq(1); + + let adminPermission = permissions[1]; + let adminRequiredAuth = adminPermission.required_auth; + chai.expect(adminPermission.parent).to.eq('one'); + chai.expect(adminPermission.perm_name).to.eq('admin'); + chai.expect(adminRequiredAuth.threshold).to.eq(1); + chai.expect(adminRequiredAuth.accounts).to.deep.equal([ + { + permission: { + actor: shared.daccustodian_contract.account.name, + permission: 'eosio.code', + }, + weight: 1, + }, + ]); + + let highPermission = permissions[2]; + let highRequiredAuth = highPermission.required_auth; + chai.expect(highPermission.parent).to.eq('active'); + chai.expect(highPermission.perm_name).to.eq('high'); + chai.expect(highRequiredAuth.threshold).to.eq(4); + + let highAccounts = highRequiredAuth.accounts; + chai.expect(highAccounts).to.deep.equal(expected_accounts); + + let lowPermission = permissions[3]; + let lowRequiredAuth = lowPermission.required_auth; + + chai.expect(lowPermission.parent).to.eq('med'); + chai.expect(lowPermission.perm_name).to.eq('low'); + chai.expect(lowRequiredAuth.threshold).to.eq(2); + + let lowAccounts = lowRequiredAuth.accounts; + chai.expect(lowAccounts).to.deep.equal(expected_accounts); + + let medPermission = permissions[4]; + let medRequiredAuth = medPermission.required_auth; + + chai.expect(medPermission.parent).to.eq('high'); + chai.expect(medPermission.perm_name).to.eq('med'); + chai.expect(medRequiredAuth.threshold).to.eq(3); + + let medAccounts = medRequiredAuth.accounts; + chai.expect(medAccounts).to.deep.equal(expected_accounts); + + let onePermission = account.permissions[5]; + let oneRequiredAuth = onePermission.required_auth; + + chai.expect(onePermission.parent).to.eq('low'); + chai.expect(onePermission.perm_name).to.eq('one'); + chai.expect(oneRequiredAuth.threshold).to.eq(1); + let oneAccounts = oneRequiredAuth.accounts; + chai.expect(oneAccounts).to.deep.equal(expected_accounts); + }); + it('running newperiod again should succeed', async () => { + await sleep(6_000); + await shared.daccustodian_contract.newperiod('new period', dacId, { + from: newUser1, + }); + }); it('custodians should have been paid', async () => { await assertRowCount( shared.daccustodian_contract.pendingpayTable({ @@ -2267,6 +2339,10 @@ describe('Daccustodian', () => { await shared.daccustodian_contract.newperiod('resigndac', dacId, { from: regMembers[0], }); + await sleep(6_000); + await shared.daccustodian_contract.newperiod('resigndac', dacId, { + from: regMembers[0], + }); }); it('should fail with incorrect auth returning auth error', async () => { let electedCandidateToResign = existing_candidates[0]; @@ -2280,56 +2356,52 @@ describe('Daccustodian', () => { }); context('with correct auth', async () => { context('for a currently elected custodian', async () => { - context('without enough elected candidates to replace', async () => { - it('should fail with not enough candidates error', async () => { - let electedCandidateToResign = existing_candidates[3]; - // The implementation of `voteForCustodians` only votes for enough to - // satisfy the config that requires 5 candidates be voted for. - // Therefore the `resigncust` would fail because a replacement candidate is not - // available until another candiate has been voted for. - await assertEOSErrorIncludesMessage( - shared.daccustodian_contract.resigncust( - electedCandidateToResign.name, - dacId, - { - auths: [ - { - actor: electedCandidateToResign.name, - permission: 'active', - }, - { - actor: shared.auth_account.name, - permission: 'active', - }, - ], - } - ), - 'NEWPERIOD_NOT_ENOUGH_CANDIDATES' + context('with enough elected candidates to replace', async () => { + let electedCandidateToResign: Account; + it('should work', async () => { + electedCandidateToResign = existing_candidates[3]; + // After this, 4 candidates will be left. This is still enough to satisfy the auth threshold. + await shared.daccustodian_contract.resigncust( + electedCandidateToResign.name, + dacId, + { + auths: [ + { + actor: electedCandidateToResign.name, + permission: 'active', + }, + { + actor: shared.auth_account.name, + permission: 'active', + }, + ], + } ); }); - context( - 'with enough elected candidates to replace a removed candidate', - async () => { - let electedCandidateToResign: Account; - before(async () => { - electedCandidateToResign = existing_candidates[3]; - await debugPromise( - shared.daccustodian_contract.votecust( - regMembers[14].name, - [ - existing_candidates[0].name, - existing_candidates[1].name, - existing_candidates[2].name, - existing_candidates[5].name, - ], - dacId, - { from: regMembers[14] } - ), - 'voting for an extra candidate' - ); - }); - it('should succeed', async () => { - await shared.daccustodian_contract.resigncust( + it('should disable candidate', async () => { + const res = await shared.daccustodian_contract.candidatesTable({ + scope: dacId, + limit: 20, + lowerBound: electedCandidateToResign.name, + upperBound: electedCandidateToResign.name, + }); + chai + .expect(res.rows[0].candidate_name) + .to.equal(electedCandidateToResign.name); + chai.expect(res.rows[0].is_active).to.equal(0); + }); + }); + + context( + 'without enough elected candidates to replace a removed candidate', + async () => { + let electedCandidateToResign: Account; + before(async () => { + electedCandidateToResign = existing_candidates[2]; + }); + it('should fail with THRESHOLD_CANNOT_BE_MET error', async () => { + await assertEOSErrorIncludesMessage( + shared.daccustodian_contract.resigncust( electedCandidateToResign.name, dacId, { @@ -2341,23 +2413,13 @@ describe('Daccustodian', () => { { actor: shared.auth_account.name, permission: 'active' }, ], } - ); - }); - it('should disable candidate', async () => { - const res = await shared.daccustodian_contract.candidatesTable({ - scope: dacId, - limit: 20, - lowerBound: electedCandidateToResign.name, - upperBound: electedCandidateToResign.name, - }); - chai - .expect(res.rows[0].candidate_name) - .to.equal(electedCandidateToResign.name); - chai.expect(res.rows[0].is_active).to.equal(0); - }); - } - ); - }); + ), + 'Invalid authority', + 'action_validate_exception' + ); + }); + } + ); }); context('for an unelected candidate', async () => { it('should fail with not current custodian error', async () => { diff --git a/contracts/daccustodian/newperiod_components.cpp b/contracts/daccustodian/newperiod_components.cpp index 6d9560f7..58bc931e 100644 --- a/contracts/daccustodian/newperiod_components.cpp +++ b/contracts/daccustodian/newperiod_components.cpp @@ -62,49 +62,53 @@ void daccustodian::assertPeriodTime(const dacglobals &globals) { globals.get_periodlength(), periodBlockCount); } -void daccustodian::allocateCustodians(bool early_election, name dac_id) { +void daccustodian::allocateCustodians(name dac_id) { - eosio::print("Configure custodians for the next period."); + // Configure custodians for the next period. + + custodians_table custodians(get_self(), dac_id.value); + pending_custodians_table pending_custs(get_self(), dac_id.value); - custodians_table custodians(get_self(), dac_id.value); candidates_table registered_candidates(get_self(), dac_id.value); const auto globals = dacglobals{get_self(), dac_id}; name auth_account = dacdir::dac_for_id(dac_id).owner; auto byvotes = registered_candidates.get_index<"bydecayed"_n>(); - auto cand_itr = byvotes.begin(); - - const auto electcount = S{globals.get_numelected()}; - auto currentCustodianCount = S{uint8_t{0}}; - auto newCustodianCount = S{uint8_t{0}}; + const auto electcount = S{globals.get_numelected()}; + auto cand_itr = byvotes.begin(); - if (!early_election) { - eosio::print("Empty the custodians table to get a full set of new custodians based on the current votes."); - auto cust_itr = custodians.begin(); + // Empty the custodians table to get a full set of new custodians based on the current votes. + auto cust_itr = custodians.begin(); + if (pending_custs.begin() != pending_custs.end()) { while (cust_itr != custodians.end()) { - const auto ®_candidate = registered_candidates.get(cust_itr->cust_name.value, - "ERR::NEWPERIOD_EXPECTED_CAND_NOT_FOUND::Corrupt data: Trying to set a lockup delay on candidate leaving office."); - registered_candidates.modify(reg_candidate, same_payer, [&](candidate &c) { - eosio::print("Lockup stake for release delay."); - }); cust_itr = custodians.erase(cust_itr); } } - - eosio::print("Select only enough candidates to fill the gaps."); - for (auto itr = custodians.begin(); itr != custodians.end(); itr++) { - currentCustodianCount++; + // Move pending custodians into custodians1 + auto pending_cust_itr = pending_custs.begin(); + while (pending_cust_itr != pending_custs.end()) { + custodians.emplace(auth_account, [&](custodian &c) { + c.cust_name = pending_cust_itr->cust_name; + c.requestedpay = pending_cust_itr->requestedpay; + c.total_vote_power = pending_cust_itr->total_vote_power; + c.rank = pending_cust_itr->rank; + c.number_voters = pending_cust_itr->number_voters; + c.avg_vote_time_stamp = pending_cust_itr->avg_vote_time_stamp; + }); + pending_cust_itr = pending_custs.erase(pending_cust_itr); } + auto currentCustodianCount = S{uint8_t{0}}; + while (currentCustodianCount < electcount) { check(cand_itr != byvotes.end() && cand_itr->total_vote_power > 0, "ERR::NEWPERIOD_NOT_ENOUGH_CANDIDATES::There are not enough eligible candidates to run new period without causing potential lock out permission structures for this DAC."); - // If the candidate is inactive or is already a custodian skip to the next one. - if (!cand_itr->is_active || custodians.find(cand_itr->candidate_name.value) != custodians.end()) { + // If the candidate is inactive or is already a pending custodian skip to the next one. + if (!cand_itr->is_active) { cand_itr++; } else { - custodians.emplace(auth_account, [&](custodian &c) { + pending_custs.emplace(auth_account, [&](custodian &c) { c.cust_name = cand_itr->candidate_name; c.requestedpay = cand_itr->requestedpay; c.total_vote_power = cand_itr->total_vote_power; @@ -113,11 +117,20 @@ void daccustodian::allocateCustodians(bool early_election, name dac_id) { c.avg_vote_time_stamp = cand_itr->avg_vote_time_stamp; }); - newCustodianCount++; currentCustodianCount++; cand_itr++; } } + + auto newCustodianCount = S{uint8_t{0}}; + + // Find how many new custodians do not exist in the current custodians. + for (auto pending_itr = pending_custs.begin(); pending_itr != pending_custs.end(); pending_itr++) { + if (custodians.find(pending_itr->cust_name.value) == custodians.end()) { + newCustodianCount++; + } + } + if (newCustodianCount >= globals.get_auth_threshold_high()) { action(permission_level{DACDIRECTORY_CONTRACT, "govmanage"_n}, DACDIRECTORY_CONTRACT, "hdlegovchg"_n, std::make_tuple(dac_id)) @@ -149,10 +162,12 @@ void daccustodian::add_auth_to_account(const name &accountToChange, const uint8_ std::sort(weights.begin(), weights.end()); } - const auto auth = eosiosystem::authority{.threshold = threshold, .keys = {}, .accounts = weights}; - action(permission_level{accountToChange, "owner"_n}, "eosio"_n, "updateauth"_n, - std::make_tuple(accountToChange, permission, parent, auth)) - .send(); + if (weights.size() > 0) { + const auto auth = eosiosystem::authority{.threshold = threshold, .keys = {}, .accounts = weights}; + action(permission_level{accountToChange, "owner"_n}, "eosio"_n, "updateauth"_n, + std::make_tuple(accountToChange, permission, parent, auth)) + .send(); + } } void daccustodian::add_all_auths(const name &accountToChange, @@ -207,8 +222,8 @@ ACTION daccustodian::claimbudget(const name &dac_id) { // percentage value is scaled by 100, so to calculate percent we need to divide by (100 * 100 == 10000) const auto allocation_for_period = treasury_balance * budget_percentage / 10000; - // if the calculated allocation_for_period is very small round it up to 10 TLM or the full treasury balance to avoid - // dust transactions for low percentage/balances in treasury. + // if the calculated allocation_for_period is very small round it up to 10 TLM or the full treasury balance to + // avoid dust transactions for low percentage/balances in treasury. const auto rounded_allocation_for_period = std::max(allocation_for_period, asset{100000, symbol{"TLM", 4}}); // Because this has been rounded up, ensure we don't attempt to transfer more than the treasury balance. @@ -298,13 +313,13 @@ ACTION daccustodian::runnewperiod(const string &message, const name &dac_id) { globals.set_met_initial_votes_threshold(true); - // Distribute Pay is called before allocateCustodians is called to ensure custodians are paid for the just passed - // period. This also implies custodians should not be paid the first time this is called. - // Distribute pay to the current custodians. + // Distribute Pay is called before allocateCustodians is called to ensure custodians are paid for the just + // passed period. This also implies custodians should not be paid the first time this is called. Distribute pay + // to the current custodians. distributeMeanPay(dac_id); // Set custodians for the next period. - allocateCustodians(false, dac_id); + allocateCustodians(dac_id); // Set the auths on the dacauthority account setMsigAuths(dac_id); diff --git a/contracts/daccustodian/registering.cpp b/contracts/daccustodian/registering.cpp index 4fe53da5..298a9ea1 100644 --- a/contracts/daccustodian/registering.cpp +++ b/contracts/daccustodian/registering.cpp @@ -168,20 +168,24 @@ void daccustodian::validateMinStake(name account, name dac_id) { void daccustodian::removeCustodian(name cust, name dac_id) { custodians_table custodians(_self, dac_id.value); - auto elected = custodians.find(cust.value); - check(elected != custodians.end(), + auto elected = custodians.require_find(cust.value, "ERR::REMOVECUSTODIAN_NOT_CURRENT_CUSTODIAN::The entered account name is not for a current custodian."); - - eosio::print("Remove custodian from the custodians table."); custodians.erase(elected); + pending_custodians_table pending_custs(get_self(), dac_id.value); + + auto pending = pending_custs.find(cust.value); + if (pending != pending_custs.end()) { + pending_custs.erase(pending); + } // Remove the candidate from being eligible for the next election period. disableCandidate(cust, dac_id); - // Allocate the next set of candidates to only fill the gap for the missing slot. - allocateCustodians(true, dac_id); + // Rather than allocate to fill the gaps leave the gap to avoid nasty suprises from newly added custodians. // Update the auths to give control to the new set of custodians. + // If too many are removed to satisfy the auth this should fail and therefore prevent the custodian to withdraw + // before the end of the period. setMsigAuths(dac_id); } diff --git a/contracts/stakevote/stakevote.test.ts b/contracts/stakevote/stakevote.test.ts index 4bf817cd..affbeff2 100644 --- a/contracts/stakevote/stakevote.test.ts +++ b/contracts/stakevote/stakevote.test.ts @@ -330,6 +330,14 @@ describe('Stakevote', () => { from: regMembers[0], // Could be run by anyone. } ); + await sleep(6_000); + await shared.daccustodian_contract.newperiod( + 'new period', + dacId, + { + from: regMembers[0], // Could be run by anyone. + } + ); await assertRowCount( shared.daccustodian_contract.custodians1Table({