Skip to content

Commit

Permalink
Merge pull request #70 from peerplays-network/GRPH-59-Proposal-failur…
Browse files Browse the repository at this point in the history 
…e-handling

 Proposal failure handling
  • Loading branch information
@bobinson
bobinson authored Sep 6, 2019
2 parents db8efaf + a797787 commit 64c05f9
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 55 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ else( WIN32 ) # Apple AND Linux
endif( APPLE )

if( "${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" )
set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-builtin-memcmp" )
set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-builtin-memcmp -Wno-class-memaccess -Wno-parentheses -Wno-terminate -Wno-invalid-offsetof" )
elseif( "${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" )
if( CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL 4.0.0 OR CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.0.0 )
set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-invalid-partial-specialization" )
Expand Down
3 changes: 2 additions & 1 deletion libraries/chain/include/graphene/chain/proposal_object.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ class proposal_object : public abstract_object<proposal_object>
flat_set<account_id_type> available_owner_approvals;
flat_set<public_key_type> available_key_approvals;
account_id_type proposer;
std::string fail_reason;

bool is_authorized_to_execute(database& db)const;
bool is_authorized_to_execute(database& db) const;
};

/**
Expand Down
5 changes: 1 addition & 4 deletions libraries/chain/include/graphene/chain/vesting_balance_object.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@
#include <algorithm>
#include <boost/multi_index/composite_key.hpp>

#define offset_d(i,f) (long(&(i)->f) - long(i))
#define offset_s(t,f) offset_d((t*)1000, f)

namespace graphene { namespace chain {
using namespace graphene::db;

Expand Down Expand Up @@ -191,7 +188,7 @@ namespace graphene { namespace chain {
member_offset<vesting_balance_object, asset_id_type, (size_t) (offsetof(vesting_balance_object,balance) + offsetof(asset,asset_id))>,
member_offset<vesting_balance_object, share_type, (size_t) (offsetof(vesting_balance_object,balance) + offsetof(asset,amount))>
//member<vesting_balance_object, account_id_type, &vesting_balance_object::owner>
//member_offset<vesting_balance_object, account_id_type, (size_t) (offset_s(vesting_balance_object,owner))>
//member_offset<vesting_balance_object, account_id_type, (size_t) (offsetof(vesting_balance_object,owner))>
>,
composite_key_compare<
std::less< asset_id_type >,
Expand Down
17 changes: 3 additions & 14 deletions libraries/chain/proposal_evaluator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,6 @@ void_result proposal_update_evaluator::do_evaluate(const proposal_update_operati
"", ("id", id)("available", _proposal->available_owner_approvals) );
}

/* All authority checks happen outside of evaluators
if( (d.get_node_properties().skip_flags & database::skip_authority_check) == 0 )
{
for( const auto& id : o.key_approvals_to_add )
{
FC_ASSERT( trx_state->signed_by(id) );
}
for( const auto& id : o.key_approvals_to_remove )
{
FC_ASSERT( trx_state->signed_by(id) );
}
}
*/

return void_result();
} FC_CAPTURE_AND_RETHROW( (o) ) }

Expand Down Expand Up @@ -293,6 +279,9 @@ void_result proposal_update_evaluator::do_apply(const proposal_update_operation&
try {
_processed_transaction = d.push_proposal(*_proposal);
} catch(fc::exception& e) {
d.modify(*_proposal, [&e](proposal_object& p) {
p.fail_reason = e.to_string(fc::log_level(fc::log_level::all));
});
wlog("Proposed transaction ${id} failed to apply once approved with exception:\n----\n${reason}\n----\nWill try again when it expires.",
("id", o.proposal)("reason", e.to_detail_string()));
_proposal_failed = true;
Expand Down
3 changes: 0 additions & 3 deletions libraries/chain/proposal_object.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,11 @@ bool proposal_object::is_authorized_to_execute(database& db) const
}
catch ( const fc::exception& e )
{
//idump((available_active_approvals));
//wlog((e.to_detail_string()));
return false;
}
return true;
}


void required_approval_index::object_inserted( const object& obj )
{
assert( dynamic_cast<const proposal_object*>(&obj) );
Expand Down
107 changes: 75 additions & 32 deletions tests/tests/authority_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ BOOST_AUTO_TEST_CASE( simple_single_signature )
sign(trx, nathan_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );

BOOST_CHECK_EQUAL(get_balance(nathan, core), old_balance - 500);
BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 500));
} catch (fc::exception& e) {
edump((e.to_detail_string()));
throw;
Expand Down Expand Up @@ -97,25 +97,25 @@ BOOST_AUTO_TEST_CASE( any_two_of_three )
GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
sign(trx, nathan_key2);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(nathan, core), old_balance - 500);
BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 500));

trx.signatures.clear();
sign(trx, nathan_key2);
sign(trx, nathan_key3);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(nathan, core), old_balance - 1000);
BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 1000));

trx.signatures.clear();
sign(trx, nathan_key1);
sign(trx, nathan_key3);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(nathan, core), old_balance - 1500);
BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 1500));

trx.signatures.clear();
//sign(trx, fc::ecc::private_key::generate());
sign(trx,nathan_key3);
GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
BOOST_CHECK_EQUAL(get_balance(nathan, core), old_balance - 1500);
BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 1500));
} catch (fc::exception& e) {
edump((e.to_detail_string()));
throw;
Expand Down Expand Up @@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
BOOST_TEST_MESSAGE( "Attempting to transfer with parent1 and parent2 signature, should succeed" );
sign(trx,parent1_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 500);
BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 500));
trx.operations.clear();
trx.signatures.clear();

Expand All @@ -180,7 +180,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
sign(trx,parent1_key);
sign(trx,parent2_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_REQUIRE_EQUAL(child.active.num_auths(), 3);
BOOST_REQUIRE_EQUAL(child.active.num_auths(), 3u);
trx.operations.clear();
trx.signatures.clear();
}
Expand All @@ -203,13 +203,13 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
BOOST_TEST_MESSAGE( "Attempting transfer both parents, should succeed" );
sign(trx, parent1_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 1000);
BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 1000));
trx.signatures.clear();

BOOST_TEST_MESSAGE( "Attempting transfer with just child key, should succeed" );
sign(trx, child_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 1500);
BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 1500));
trx.operations.clear();
trx.signatures.clear();

Expand Down Expand Up @@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )

BOOST_TEST_MESSAGE( "Attempt to transfer using parent2_key and grandparent_key" );
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 2000);
BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 2000));
trx.clear();

BOOST_TEST_MESSAGE( "Update grandparent account authority to be committee account" );
Expand All @@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
trx.signatures.clear();
sign(trx, child_key);
PUSH_TX( db, trx, database::skip_transaction_dupe_check );
BOOST_CHECK_EQUAL(get_balance(child, core), old_balance - 2500);
BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 2500));
trx.operations.clear();
trx.signatures.clear();

Expand Down Expand Up @@ -329,17 +329,17 @@ BOOST_AUTO_TEST_CASE( proposed_single_account )
vector<authority> other;
flat_set<account_id_type> active_set, owner_set;
operation_get_required_authorities(op,active_set,owner_set,other);
BOOST_CHECK_EQUAL(active_set.size(), 1);
BOOST_CHECK_EQUAL(owner_set.size(), 0);
BOOST_CHECK_EQUAL(other.size(), 0);
BOOST_CHECK_EQUAL(active_set.size(), 1lu);
BOOST_CHECK_EQUAL(owner_set.size(), 0lu);
BOOST_CHECK_EQUAL(other.size(), 0lu);
BOOST_CHECK(*active_set.begin() == moneyman.get_id());

active_set.clear();
other.clear();
operation_get_required_authorities(op.proposed_ops.front().op,active_set,owner_set,other);
BOOST_CHECK_EQUAL(active_set.size(), 1);
BOOST_CHECK_EQUAL(owner_set.size(), 0);
BOOST_CHECK_EQUAL(other.size(), 0);
BOOST_CHECK_EQUAL(active_set.size(), 1lu);
BOOST_CHECK_EQUAL(owner_set.size(), 0lu);
BOOST_CHECK_EQUAL(other.size(), 0lu);
BOOST_CHECK(*active_set.begin() == nathan.id);
}

Expand All @@ -349,10 +349,10 @@ BOOST_AUTO_TEST_CASE( proposed_single_account )
sign( trx, init_account_priv_key );
const proposal_object& proposal = db.get<proposal_object>(PUSH_TX( db, trx ).operation_results.front().get<object_id_type>());

BOOST_CHECK_EQUAL(proposal.required_active_approvals.size(), 1);
BOOST_CHECK_EQUAL(proposal.available_active_approvals.size(), 0);
BOOST_CHECK_EQUAL(proposal.required_owner_approvals.size(), 0);
BOOST_CHECK_EQUAL(proposal.available_owner_approvals.size(), 0);
BOOST_CHECK_EQUAL(proposal.required_active_approvals.size(), 1lu);
BOOST_CHECK_EQUAL(proposal.available_active_approvals.size(), 0lu);
BOOST_CHECK_EQUAL(proposal.required_owner_approvals.size(), 0lu);
BOOST_CHECK_EQUAL(proposal.available_owner_approvals.size(), 0lu);
BOOST_CHECK(*proposal.required_active_approvals.begin() == nathan.id);

proposal_update_operation pup;
Expand Down Expand Up @@ -389,6 +389,49 @@ BOOST_AUTO_TEST_CASE( proposed_single_account )
}
}

BOOST_AUTO_TEST_CASE( proposal_failure )
{
try
{
ACTORS( (bob) (alice) );

fund( bob, asset(1000000) );
fund( alice, asset(1000000) );

// create proposal that will eventually fail due to lack of funds
transfer_operation top;
top.to = alice_id;
top.from = bob_id;
top.amount = asset(2000000);
proposal_create_operation pop;
pop.proposed_ops.push_back( { top } );
pop.expiration_time = db.head_block_time() + fc::days(1);
pop.fee_paying_account = bob_id;
trx.operations.push_back( pop );
trx.signatures.clear();
sign( trx, bob_private_key );
processed_transaction processed = PUSH_TX( db, trx );
proposal_object prop = db.get<proposal_object>(processed.operation_results.front().get<object_id_type>());
trx.clear();
generate_block();
// add signature
proposal_update_operation up_op;
up_op.proposal = prop.id;
up_op.fee_paying_account = bob_id;
up_op.active_approvals_to_add.emplace( bob_id );
trx.operations.push_back( up_op );
sign( trx, bob_private_key );
PUSH_TX( db, trx );
trx.clear();

// check fail reason
const proposal_object& result = db.get<proposal_object>(prop.id);
BOOST_CHECK(!result.fail_reason.empty());
BOOST_CHECK_EQUAL( result.fail_reason.substr(0, 16), "Assert Exception");
}
FC_LOG_AND_RETHROW()
}

/// Verify that committee authority cannot be invoked in a normal transaction
BOOST_AUTO_TEST_CASE( committee_authority )
{ try {
Expand Down Expand Up @@ -696,15 +739,15 @@ BOOST_FIXTURE_TEST_CASE( proposal_delete, database_fixture )
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_active_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.available_active_approvals.size(), 1lu);

std::swap(uop.active_approvals_to_add, uop.active_approvals_to_remove);
trx.operations.push_back(uop);
sign( trx, nathan_key );
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_active_approvals.size(), 0);
BOOST_CHECK_EQUAL(prop.available_active_approvals.size(), 0lu);
}

{
Expand Down Expand Up @@ -758,8 +801,8 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_delete, database_fixture )
}

const proposal_object& prop = *db.get_index_type<proposal_index>().indices().begin();
BOOST_CHECK_EQUAL(prop.required_active_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.required_owner_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.required_active_approvals.size(), 1lu);
BOOST_CHECK_EQUAL(prop.required_owner_approvals.size(), 1lu);
BOOST_CHECK(!prop.is_authorized_to_execute(db));

{
Expand All @@ -772,15 +815,15 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_delete, database_fixture )
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_owner_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.available_owner_approvals.size(), 1lu);

std::swap(uop.owner_approvals_to_add, uop.owner_approvals_to_remove);
trx.operations.push_back(uop);
sign( trx, nathan_key );
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_owner_approvals.size(), 0);
BOOST_CHECK_EQUAL(prop.available_owner_approvals.size(), 0lu);
}

{
Expand Down Expand Up @@ -835,8 +878,8 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_complete, database_fixture )
}

const proposal_object& prop = *db.get_index_type<proposal_index>().indices().begin();
BOOST_CHECK_EQUAL(prop.required_active_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.required_owner_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.required_active_approvals.size(), 1lu);
BOOST_CHECK_EQUAL(prop.required_owner_approvals.size(), 1lu);
BOOST_CHECK(!prop.is_authorized_to_execute(db));

{
Expand All @@ -852,7 +895,7 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_complete, database_fixture )
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 1lu);

std::swap(uop.key_approvals_to_add, uop.key_approvals_to_remove);
trx.operations.push_back(uop);
Expand All @@ -862,7 +905,7 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_complete, database_fixture )
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 0);
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 0lu);

std::swap(uop.key_approvals_to_add, uop.key_approvals_to_remove);
trx.operations.push_back(uop);
Expand All @@ -872,7 +915,7 @@ BOOST_FIXTURE_TEST_CASE( proposal_owner_authority_complete, database_fixture )
PUSH_TX( db, trx );
trx.clear();
BOOST_CHECK(!prop.is_authorized_to_execute(db));
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 1);
BOOST_CHECK_EQUAL(prop.available_key_approvals.size(), 1lu);

uop.key_approvals_to_add.clear();
uop.owner_approvals_to_add.insert(nathan.get_id());
Expand Down

0 comments on commit 64c05f9

Please sign in to comment.