From 8e9bd890a8fc1af7a951e57042dc14be85133908 Mon Sep 17 00:00:00 2001
From: Eric Frias <efrias@syncad.com>
Date: Thu, 10 Sep 2015 19:15:41 -0400
Subject: [PATCH] Fix bugs, improve logging in p2p sync

---
 libraries/app/application.cpp | 20 +++++++++++---------
 libraries/net/node.cpp        |  8 +++++---
 tests/tests/block_tests.cpp   |  6 +++---
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/libraries/app/application.cpp b/libraries/app/application.cpp
index 2f546492..1f83b3c7 100644
--- a/libraries/app/application.cpp
+++ b/libraries/app/application.cpp
@@ -27,6 +27,7 @@
 #include <graphene/egenesis/egenesis.hpp>
 
 #include <graphene/net/core_messages.hpp>
+#include <graphene/net/exceptions.hpp>
 
 #include <graphene/time/time.hpp>
 
@@ -605,6 +606,14 @@ namespace detail {
                 throw;
               }
             }
+            if (non_fork_high_block_num < low_block_num)
+            {
+              wlog("Unable to generate a usable synopsis because the peer we're generating it for forked too long ago "
+                   "(our chains diverge after block #${non_fork_high_block_num} but only undoable to block #${low_block_num})", 
+                   ("low_block_num", low_block_num)
+                   ("non_fork_high_block_num", non_fork_high_block_num));
+              FC_THROW_EXCEPTION(graphene::net::block_older_than_undo_history, "Peer is are on a fork I'm unable to switch to");
+            }
           }
           else
           {
@@ -620,15 +629,8 @@ namespace detail {
           // non_fork_high_block_num is the block before the fork (if the peer is on a fork, or otherwise it is the same as high_block_num)
           // high_block_num is the block number of the reference block, or the end of the chain if no reference provided
 
-          if (non_fork_high_block_num <= low_block_num)
-          {
-            wlog("Unable to generate a usable synopsis because the peer we're generating it for forked too long ago "
-                 "(our chains diverge after block #${non_fork_high_block_num} but only undoable to block #${low_block_num})", 
-                 ("low_block_num", low_block_num)
-                 ("non_fork_high_block_num", non_fork_high_block_num));
-            return synopsis;
-          }
-
+          // true_high_block_num is the ending block number after the network code appends any item ids it 
+          // knows about that we don't
           uint32_t true_high_block_num = high_block_num + number_of_blocks_after_reference_point;
           do
           {
diff --git a/libraries/net/node.cpp b/libraries/net/node.cpp
index e3f75360..f4cc31be 100644
--- a/libraries/net/node.cpp
+++ b/libraries/net/node.cpp
@@ -1321,9 +1321,9 @@ namespace graphene { namespace net { namespace detail {
                 active_peer->item_ids_requested_from_peer &&
                 active_peer->item_ids_requested_from_peer->get<1>() < active_ignored_request_threshold)
               {
-                wlog("Disconnecting peer ${peer} because they didn't respond to my request for sync item ids after ${id}",
+                wlog("Disconnecting peer ${peer} because they didn't respond to my request for sync item ids after ${synopsis}",
                       ("peer", active_peer->get_remote_endpoint())
-                      ("id", active_peer->item_ids_requested_from_peer->get<0>().back()));
+                      ("synopsis", active_peer->item_ids_requested_from_peer->get<0>()));
                 disconnect_due_to_request_timeout = true;
               }
             if (!disconnect_due_to_request_timeout)
@@ -2269,6 +2269,7 @@ namespace graphene { namespace net { namespace detail {
 
       std::vector<item_hash_t> synopsis = _delegate->get_blockchain_synopsis(reference_point, number_of_blocks_after_reference_point);
 
+#if 0
       // just for debugging, enable this and set a breakpoint to step through
       if (synopsis.empty())
         synopsis = _delegate->get_blockchain_synopsis(reference_point, number_of_blocks_after_reference_point);
@@ -2277,8 +2278,9 @@ namespace graphene { namespace net { namespace detail {
       // or if the reference point is now past our undo history (that's not). 
       // in the second case, we should mark this peer as one we're unable to sync with and
       // disconnect them.
-      if (reference_point != item_hash_t() &&  synopsis.empty())
+      if (reference_point != item_hash_t() && synopsis.empty())
         FC_THROW_EXCEPTION(block_older_than_undo_history, "You are on a fork I'm unable to switch to");
+#endif
 
       if( number_of_blocks_after_reference_point )
       {
diff --git a/tests/tests/block_tests.cpp b/tests/tests/block_tests.cpp
index 4a904390..f0830d48 100644
--- a/tests/tests/block_tests.cpp
+++ b/tests/tests/block_tests.cpp
@@ -1076,7 +1076,7 @@ BOOST_FIXTURE_TEST_CASE( transaction_invalidated_in_cache, database_fixture )
          }
       };
 
-      auto generate_xfer_tx = [&]( account_id_type from, account_id_type to, share_type amount, int blocks_to_expire=10 ) -> signed_transaction
+      auto generate_xfer_tx = [&]( account_id_type from, account_id_type to, share_type amount, int blocks_to_expire ) -> signed_transaction
       {
          signed_transaction tx;
          transfer_operation xfer_op;
@@ -1128,8 +1128,8 @@ BOOST_FIXTURE_TEST_CASE( transaction_invalidated_in_cache, database_fixture )
       //
 
       signed_transaction tx_a = generate_xfer_tx( bob_id, alice_id, 1000, 3 );
-      signed_transaction tx_b = generate_xfer_tx( alice_id, bob_id, 2000 );
-      signed_transaction tx_c = generate_xfer_tx( alice_id, bob_id,  500 );
+      signed_transaction tx_b = generate_xfer_tx( alice_id, bob_id, 2000, 10 );
+      signed_transaction tx_c = generate_xfer_tx( alice_id, bob_id,  500, 10 );
 
       generate_block( db );