Log successful AcceptBlockHeader() #27276

1. 
   Sjors commented at 12:20 pm on March 19, 2023: member

   Knowing when a header was first seen may help distinguish between a regular reorg and a selfish mining or eclipse attack.
2. 
   DrahtBot commented at 12:20 pm on March 19, 2023: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	dergoegge

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. in src/validation.cpp:3831 in a6f9809bfb outdated

   3827@@ -3828,6 +3828,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
   3828         return state.Invalid(BlockValidationResult::BLOCK_HEADER_LOW_WORK, "too-little-chainwork");
   3829     }
   3830     CBlockIndex* pindex{m_blockman.AddToBlockIndex(block, m_best_header)};
   3831+    LogPrint(BCLog::VALIDATION, "%s: added new block header %s\n", __func__, hash.ToString());
   

   ---

   ---

   

   dergoegge commented at 1:10 pm on March 19, 2023:

   0    LogPrint(BCLog::VALIDATION, "added new block header %s\n", hash.ToString());
   

   We have an option for logging source locations -logsourcelocations.

   ---

   

   Sjors commented at 1:24 pm on March 19, 2023:

   Done!
4. 
   dergoegge commented at 1:10 pm on March 19, 2023: member

   Concept ACK
5. Sjors force-pushed on Mar 19, 2023
6. 
   jamesob commented at 1:55 pm on March 19, 2023: member

   Beat me to it! I was just about to open this PR :).

   For what it’s worth, I think that the right approach here is to make this message LogPrintf() and omit it when in initial block download, the rationale being that if something weird happens on the network (like the inspiration for this PR last night), having a lot of data available to investigate would be nice, and most people don’t run with verbose logging. Since the additional log volume here is quite minimal (one extra line per block, basically), I don’t think there’s much cost.

7. 
   Sjors commented at 2:02 pm on March 19, 2023: member

   I opened #27277 to make -debug=validation less noisy, making it a more attractive setting. At least after IBD, since it produces ~4 lines per block: AcceptBlockHeader, NewPoWValidBlock, Pre-allocating and BlockChecked.

   (just pushed another commit there to move Pre-allocating to the blockstorage category)

8. 
   fanquake commented at 2:03 pm on March 19, 2023: member

   cc @0xB10C
9. Log successful AcceptBlockHeader()

   Knowing when a header was first seen may help distinguish between a regular reorg and a selfish mining or eclipse attack.

   3d32b7d440
10. in src/validation.cpp:3831 in c8d044e8fd outdated

    3827@@ -3828,6 +3828,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
    3828         return state.Invalid(BlockValidationResult::BLOCK_HEADER_LOW_WORK, "too-little-chainwork");
    3829     }
    3830     CBlockIndex* pindex{m_blockman.AddToBlockIndex(block, m_best_header)};
    3831+    LogPrint(BCLog::VALIDATION, "added new block header %s\n", hash.ToString());
    

    ---

    ---

    

    jonatack commented at 2:40 pm on March 19, 2023:

    LogPrint is being migrated to LogPrintLevel – you can use that when adding logging. Or LogPrintfCategory if you decide to make this logged unconditionally, but the former with a level of, say, info, may suffice; example below.

    0    LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Info, "added new block header %s\n", hash.ToString());
    

    ---

    

    Sjors commented at 3:25 pm on March 19, 2023:

    Done
11. Sjors force-pushed on Mar 19, 2023
12. 
    Sjors commented at 3:42 pm on March 19, 2023: member

    At this point I prefer #27278, but I’ll leave this open in case that PR doesn’t make it. This one is (slightly) easier to review.
13. 
    fanquake commented at 11:00 am on March 20, 2023: member

    Closing in favour of #27278.
14. fanquake closed this on Mar 20, 2023

    ---

15. achow101 referenced this in commit 664500fc71 on Mar 21, 2023
16. sidhujag referenced this in commit 4b54cd3f6c on Mar 21, 2023
17. bitcoin locked this on Mar 19, 2024

Contributors
Sjors DrahtBot dergoegge jamesob fanquake jonatack