[GRPH-59] Proposal failure handling

CMakeLists.txt

@@ -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" )

libraries/chain/include/graphene/chain/proposal_object.hpp

@@ -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;
 };
 
 /**

libraries/chain/include/graphene/chain/vesting_balance_object.hpp

@@ -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;
 
@@ -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 >,

libraries/chain/proposal_evaluator.cpp

@@ -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) ) }
 
@@ -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;

libraries/chain/proposal_object.cpp

@@ -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) );

tests/tests/authority_tests.cpp

@@ -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;
@@ -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;
@@ -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();
 
@@ -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();
       }
@@ -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();
 
@@ -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" );
@@ -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();
 
@@ -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);
       }
 
@@ -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;
@@ -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 {
@@ -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);
    }
 
    {
@@ -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));
 
    {
@@ -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);
    }
 
    {
@@ -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));
 
    {
@@ -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);
@@ -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);
@@ -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());