Skip to content

Commit

Permalink
Merge pull request #81 from Alien-Worlds/DEV-2690_disallow-duplicate-…
Browse files Browse the repository at this point in the history 
…approvals

Dev 2690 disallow duplicate approvals
  • Loading branch information
@angelol
angelol authored Nov 1, 2022
2 parents 2116ccc + 6203ca3 commit 54568d0
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 15 deletions.
28 changes: 16 additions & 12 deletions contracts/msigworlds/msigworlds.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void multisig::propose(name proposer, name proposal_name, std::vector<permission

auto packed_requested = pack(requested);
auto res = check_transaction_authorization(
trx_pos, size, (const char *)0, 0, packed_requested.data(), packed_requested.size());
trx_pos, size, (const char *)0, 0, packed_requested.data(), packed_requested.size());

check(res > 0, "msigworlds::propose preflight transaction authorization check failed");

Expand Down Expand Up @@ -99,26 +99,30 @@ void multisig::approve(
}

proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "proposal not found");
auto &prop = proptable.get(proposal_name.value, "proposal not found");
check(prop.state == PropState::PENDING,
"ERR::PROP_NOT_PENDING::proposal can only be approved while in pending state");

if (proposal_hash.has_value()) {
assert_sha256(prop.packed_transaction.data(), prop.packed_transaction.size(), proposal_hash.value());
}

const auto this_approval = approval{level, current_time_point()};

approvals apptable(get_self(), dac_id.value);
auto apps_it = apptable.require_find(proposal_name.value, "ERR::NO_APPROVALS_FOUND::No approvals were found.");
auto itr =
std::find_if(apps_it->requested_approvals.begin(), apps_it->requested_approvals.end(), [&](const approval &a) {
return a.level == level;
});
auto itr = std::find(apps_it->requested_approvals.begin(), apps_it->requested_approvals.end(), this_approval);

auto is_requested_approval = itr != apps_it->requested_approvals.end();
eosio::name ram_payer = is_requested_approval ? same_payer : level.actor;

apptable.modify(apps_it, ram_payer, [&](auto &a) {
a.provided_approvals.push_back(approval{level, current_time_point()});
const bool is_duplicate = std::find(a.provided_approvals.begin(), a.provided_approvals.end(), this_approval) !=
a.provided_approvals.end();
check(
!is_duplicate, "ERR::DUPLICATE_APPROVAL::Approval by %s@%s already exists.", level.actor, level.permission);

a.provided_approvals.push_back(this_approval);
if (is_requested_approval) {
a.requested_approvals.erase(itr);
}
Expand Down Expand Up @@ -179,7 +183,7 @@ void multisig::_unapprove(
}

proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "proposal not found");
auto &prop = proptable.get(proposal_name.value, "proposal not found");
check(prop.state == PropState::PENDING,
"ERR::PROP_NOT_PENDING::proposal can only be changed while in pending state.");

Expand All @@ -201,7 +205,7 @@ void multisig::_unapprove(

void multisig::checkauth(name proposal_name, name dac_id) {
proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "proposal not found");
auto &prop = proptable.get(proposal_name.value, "proposal not found");
auto table_op = [](auto &&, auto &&) {};
if (trx_is_authorized(
get_approvals_and_adjust_table(get_self(), proposal_name, table_op, dac_id), prop.packed_transaction)) {
Expand All @@ -215,7 +219,7 @@ void multisig::cancel(name proposal_name, name canceler, name dac_id) {
require_auth(canceler);

proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "proposal not found");
auto &prop = proptable.get(proposal_name.value, "proposal not found");

if (canceler != prop.proposer) {
check(unpack<transaction_header>(prop.packed_transaction).expiration <
Expand All @@ -236,7 +240,7 @@ void multisig::exec(name proposal_name, name executer, name dac_id) {
require_auth(executer);

proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "proposal not found");
auto &prop = proptable.get(proposal_name.value, "proposal not found");
check(prop.state == PropState::PENDING,
"ERR::PROP_EXEC_NOT_PENDING::The same proposal cannot be executed mulitple times.");

Expand Down Expand Up @@ -275,7 +279,7 @@ void multisig::exec(name proposal_name, name executer, name dac_id) {

void multisig::cleanup(name proposal_name, name dac_id) {
proposals proptable(get_self(), dac_id.value);
auto & prop = proptable.get(proposal_name.value, "ERR::PROPOSAL_NOT_FOUND::proposal not found");
auto &prop = proptable.get(proposal_name.value, "ERR::PROPOSAL_NOT_FOUND::proposal not found");
check(prop.state != PropState::PENDING,
"ERR::PROPOSAL_CLEANUP_STILL_PENDING::proposal cannot be cleared before being executed or cancelled.");

Expand Down
9 changes: 7 additions & 2 deletions contracts/msigworlds/msigworlds.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,17 @@ typedef eosio::multi_index<"proposals"_n, proposal,
struct approval {
permission_level level;
time_point time;

friend bool operator==(const approval &a, const approval &b) {
// approvals are considered equal if their levels match, regardless of time
return a.level == b.level;
}
};

struct [[eosio::table("approvals"), eosio::contract("msigworlds")]] approvals_info {
name proposal_name;
// requested approval doesn't need to cointain time, but we want requested approval
// to be of exact the same size ad provided approval, in this case approve/unapprove
// requested approval doesn't need to contain time, but we want requested approval
// to be of exact the same size as provided approval, in this case approve/unapprove
// doesn't change serialized data size. So, we use the same type.
std::vector<approval> requested_approvals;
std::vector<approval> provided_approvals;
Expand Down
16 changes: 15 additions & 1 deletion contracts/msigworlds/msigworlds.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const currentHeadTimeWithAddedSeconds = async (seconds: number) => {
return date;
};

describe('msigworlds', () => {
describe('Msigworlds', () => {
let shared: SharedTestObjects;

before(async () => {
Expand Down Expand Up @@ -506,6 +506,20 @@ describe('msigworlds', () => {
const props = await msigworlds.proposalsTable({ scope: dac_id });
});
});
context('duplicate', async () => {
it('should fail', async () => {
await assertEOSErrorIncludesMessage(
msigworlds.approve(
'prop1',
{ actor: owner2.name, permission: 'active' },
dac_id,
null,
{ from: owner2 }
),
'ERR::DUPLICATE_APPROVAL::'
);
});
});
});
});
context('unapprove', async () => {
Expand Down

0 comments on commit 54568d0

Please sign in to comment.