database: Undo and re-apply pending_transactions before/after pushing a block. Fixes #299

libraries/chain/db_block.cpp

@@ -17,6 +17,7 @@
  */
 
 #include <graphene/chain/database.hpp>
+#include <graphene/chain/db_with.hpp>
 
 #include <graphene/chain/block_summary_object.hpp>
 #include <graphene/chain/global_property_object.hpp>
@@ -85,9 +86,13 @@ bool database::push_block(const signed_block& new_block, uint32_t skip)
 {
    idump((new_block.block_num())(new_block.id()));
    bool result;
-   with_skip_flags(skip, [&]()
+   detail::with_skip_flags( *this, skip, [&]()
    {
-      result = _push_block(new_block);
+      detail::without_pending_transactions( *this, std::move(_pending_block.transactions),
+      [&]()
+      {
+         result = _push_block(new_block);
+      });
    });
    return result;
 }
@@ -156,11 +161,6 @@ bool database::_push_block(const signed_block& new_block)
       }
    }
 
-   // If there is a pending block session, then the database state is dirty with pending transactions.
-   // Drop the pending session to reset the database to a clean head block state.
-   // TODO: Preserve pending transactions, and re-apply any which weren't included in the new block.
-   clear_pending();
-
    try {
       auto session = _undo_db.start_undo_session();
       apply_block(new_block, skip);
@@ -187,7 +187,7 @@ bool database::_push_block(const signed_block& new_block)
 processed_transaction database::push_transaction( const signed_transaction& trx, uint32_t skip )
 { try {
    processed_transaction result;
-   with_skip_flags( skip, [&]()
+   detail::with_skip_flags( *this, skip, [&]()
    {
       result = _push_transaction( trx );
    } );
@@ -249,7 +249,7 @@ signed_block database::generate_block(
    )
 {
    signed_block result;
-   with_skip_flags( skip, [&]()
+   detail::with_skip_flags( *this, skip, [&]()
    {
       result = _generate_block( when, witness_id, block_signing_private_key, true );
    } );
@@ -390,7 +390,7 @@ void database::apply_block( const signed_block& next_block, uint32_t skip )
          skip = ~0;// WE CAN SKIP ALMOST EVERYTHING
    }
 
-   with_skip_flags( skip, [&]()
+   detail::with_skip_flags( *this, skip, [&]()
    {
       _apply_block( next_block );
    } );
@@ -479,7 +479,7 @@ void database::notify_changed_objects()
 processed_transaction database::apply_transaction(const signed_transaction& trx, uint32_t skip)
 {
    processed_transaction result;
-   with_skip_flags(skip, [&]()
+   detail::with_skip_flags( *this, skip, [&]()
    {
       result = _apply_transaction(trx);
    });

libraries/chain/db_update.cpp

@@ -17,6 +17,7 @@
  */
 
 #include <graphene/chain/database.hpp>
+#include <graphene/chain/db_with.hpp>
 
 #include <graphene/chain/asset_object.hpp>
 #include <graphene/chain/global_property_object.hpp>
@@ -100,10 +101,6 @@ void database::update_pending_block(const signed_block& next_block, uint8_t curr
 {
    _pending_block.timestamp = next_block.timestamp + current_block_interval;
    _pending_block.previous = next_block.id();
-   auto old_pending_trx = std::move(_pending_block.transactions);
-   _pending_block.transactions.clear();
-   for( auto old_trx : old_pending_trx )
-      push_transaction( old_trx );
 }
 
 void database::clear_expired_transactions()
@@ -142,7 +139,7 @@ void database::clear_expired_proposals()
 
 void database::clear_expired_orders()
 {
-   with_skip_flags(
+   detail::with_skip_flags( *this,
       get_node_properties().skip_flags | skip_authority_check, [&](){
          transaction_evaluation_state cancel_context(this);
 
@@ -158,7 +155,6 @@ void database::clear_expired_orders()
          }
      });
 
-
    //Process expired force settlement orders
    auto& settlement_index = get_index_type<force_settlement_index>().indices().get<by_expiration>();
    if( !settlement_index.empty() )

libraries/chain/include/graphene/chain/database.hpp

@@ -40,31 +40,6 @@ namespace graphene { namespace chain {
    using graphene::db::abstract_object;
    using graphene::db::object;
 
-   namespace detail
-   {
-      /**
-       * Class used to help the with_skip_flags implementation.
-       * It must be defined in this header because it must be
-       * available to the with_skip_flags implementation,
-       * which is a template and therefore must also be defined
-       * in this header.
-       */
-      struct skip_flags_restorer
-      {
-         skip_flags_restorer( node_property_object& npo, uint32_t old_skip_flags )
-            : _npo( npo ), _old_skip_flags( old_skip_flags )
-         {}
-
-         ~skip_flags_restorer()
-         {
-            _npo.skip_flags = _old_skip_flags;
-         }
-
-         node_property_object& _npo;
-         uint32_t _old_skip_flags;
-      };
-   }
-
    /**
     *   @class database
     *   @brief tracks the blockchain state in an extensible manner
@@ -270,23 +245,6 @@ namespace graphene { namespace chain {
 
          node_property_object& node_properties();
 
-         /**
-          * Set the skip_flags to the given value, call callback,
-          * then reset skip_flags to their previous value after
-          * callback is done.
-          */
-         template< typename Lambda >
-         void with_skip_flags(
-            uint32_t skip_flags,
-            Lambda callback )
-         {
-            node_property_object& npo = node_properties();
-            detail::skip_flags_restorer restorer( npo, npo.skip_flags );
-            npo.skip_flags = skip_flags;
-            callback();
-            return;
-         }
-
          //////////////////// db_init.cpp ////////////////////
 
          void initialize_evaluators();

libraries/chain/include/graphene/chain/db_with.hpp

@@ -0,0 +1,126 @@
+/*
+ * Copyright (c) 2015, Cryptonomex, Inc.
+ * All rights reserved.
+ *
+ * This source code is provided for evaluation in private test networks only, until September 8, 2015. After this date, this license expires and
+ * the code may not be used, modified or distributed for any purpose. Redistribution and use in source and binary forms, with or without modification,
+ * are permitted until September 8, 2015, provided that the following conditions are met:
+ *
+ * 1. The code and/or derivative works are used only for private test networks consisting of no more than 10 P2P nodes.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#pragma once
+
+#include <graphene/chain/database.hpp>
+
+/*
+ * This file provides with() functions which modify the database
+ * temporarily, then restore it.  These functions are mostly internal
+ * implementation detail of the database.
+ *
+ * Essentially, we want to be able to use "finally" to restore the
+ * database regardless of whether an exception is thrown or not, but there
+ * is no "finally" in C++.  Instead, C++ requires us to create a struct
+ * and put the finally block in a destructor.  Aagh!
+ */
+
+namespace graphene { namespace chain { namespace detail {
+/**
+ * Class used to help the with_skip_flags implementation.
+ * It must be defined in this header because it must be
+ * available to the with_skip_flags implementation,
+ * which is a template and therefore must also be defined
+ * in this header.
+ */
+struct skip_flags_restorer
+{
+   skip_flags_restorer( node_property_object& npo, uint32_t old_skip_flags )
+      : _npo( npo ), _old_skip_flags( old_skip_flags )
+   {}
+
+   ~skip_flags_restorer()
+   {
+      _npo.skip_flags = _old_skip_flags;
+   }
+
+   node_property_object& _npo;
+   uint32_t _old_skip_flags;
+};
+
+/**
+ * Class used to help the without_pending_transactions
+ * implementation.
+ */
+struct pending_transactions_restorer
+{
+   pending_transactions_restorer( database& db, std::vector<processed_transaction>&& pending_transactions )
+      : _db(db), _pending_transactions( std::move(pending_transactions) )
+   {
+      _db.clear_pending();
+   }
+
+   ~pending_transactions_restorer()
+   {
+      for( const processed_transaction& tx : _pending_transactions )
+      {
+         try
+         {
+            // since push_transaction() takes a signed_transaction,
+            // the operation_results field will be ignored.
+            _db.push_transaction( tx );
+         }
+         catch( const fc::exception& e )
+         {
+            wlog( "Pending transaction became invalid after switching to block ${b}", ("b", _db.head_block_id()) );
+            wlog( "The invalid pending transaction is ${t}", ("t", tx) );
+            wlog( "The invalid pending transaction caused exception ${e}", ("e", e) );
+         }
+      }
+   }
+
+   database& _db;
+   std::vector< processed_transaction > _pending_transactions;
+};
+
+/**
+ * Set the skip_flags to the given value, call callback,
+ * then reset skip_flags to their previous value after
+ * callback is done.
+ */
+template< typename Lambda >
+void with_skip_flags(
+   database& db,
+   uint32_t skip_flags,
+   Lambda callback )
+{
+   node_property_object& npo = db.node_properties();
+   skip_flags_restorer restorer( npo, npo.skip_flags );
+   npo.skip_flags = skip_flags;
+   callback();
+   return;
+}
+
+/**
+ * Empty pending_transactions, call callback,
+ * then reset pending_transactions after callback is done.
+ *
+ * Pending transactions which no longer validate will be culled.
+ */
+template< typename Lambda >
+void without_pending_transactions(
+   database& db,
+   std::vector<processed_transaction>&& pending_transactions,
+   Lambda callback )
+{
+    pending_transactions_restorer restorer( db, std::move(pending_transactions) );
+    callback();
+    return;
+}
+
+} } } // graphene::chain::detail