nochowderforyou / clams

Clam Project
MIT License
62 stars 58 forks source link

SEGV in Checkpoints::GetTotalBlocksEstimate() at first peer connect (testnet) #136

Closed bluen closed 8 years ago

bluen commented 9 years ago

Hi,

A comment in the code (checkpoints.cpp - see below) states "TestNet has no checkpoints", but Checkpoints::GetTotalBlocksEstimate() does mapCheckpointsTestnet.rbegin()->first - which dereferences the iterator returned by rbegin(). But: "If the container is empty, the returned iterator value shall not be dereferenced."

I tried to run two instances of clamd (on two separate hosts) with "-testnet", and have one of them connect to the other (with connect=.... or addnode=....). Both instances instantly crash with a SIGSEGV after the version message is received.

This modification avoids the crash (by adding one checkpoint - the testnet genesis block):

diff --git a/src/checkpoints.cpp b/src/checkpoints.cpp
index 6d13ac6..3d32461 100644
--- a/src/checkpoints.cpp
+++ b/src/checkpoints.cpp
@@ -36,7 +36,10 @@ namespace Checkpoints
     ;

-    // TestNet has no checkpoints
-    static MapCheckpoints mapCheckpointsTestnet;
+    static MapCheckpoints mapCheckpointsTestnet =
+        boost::assign::map_list_of
+        ( 0,      hashGenesisBlockTestNet )
+    ;

     bool CheckHardened(int nHeight, const uint256& hash)

I'm not sure if this is an appropriate solution - maybe simply checking if the map is empty and handle that case appropriately in the 3 occurrences of mapCheckpointsTestnet in checkpoints.cpp would be better...

Best regards

creativecuriosity commented 8 years ago

Eventually handled via: https://github.com/nochowderforyou/clams/pull/230 ?

bluen commented 8 years ago

As far as I can see handled there.

creativecuriosity commented 8 years ago

It was a long time in coming; thanks for the good work :)