Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fail_reason to proposal_object #1036

Merged
merged 4 commits into from
Jun 12, 2018
Merged

Add fail_reason to proposal_object #1036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9d54a5c
Add fail_reason to proposal object
jmjatlanta Jun 8, 2018
6df2f4b
Fixed compiler warnings signed/unsigned
jmjatlanta Jun 8, 2018
b65b518
Re-implemented solution
jmjatlanta Jun 11, 2018
ef8cfba
Test cleanup
jmjatlanta Jun 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libraries/chain/include/graphene/chain/proposal_object.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,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
17 changes: 3 additions & 14 deletions libraries/chain/proposal_evaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,20 +129,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 @@ -178,6 +164,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
Open in desktop
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
109 changes: 77 additions & 32 deletions tests/tests/authority_tests.cpp
View file
Open in desktop
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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do all these (unrelated) casts fix compiler warnings, or what is the reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, compiler warnings.

} 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 @@ -479,6 +522,8 @@ BOOST_AUTO_TEST_CASE( committee_authority )

generate_blocks(prop.expiration_time);
BOOST_CHECK_EQUAL(get_balance(nathan, asset_id_type()(db)), 100000);
// proposal deleted
BOOST_CHECK_THROW( db.get<proposal_object>(prop.id), fc::exception );
} FC_LOG_AND_RETHROW() }

BOOST_FIXTURE_TEST_CASE( fired_committee_members, database_fixture )
Expand Down Expand Up @@ -693,15 +738,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 @@ -755,8 +800,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 @@ -769,15 +814,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 @@ -832,8 +877,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 @@ -849,7 +894,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 @@ -859,7 +904,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 @@ -869,7 +914,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