nochowderforyou / clams

Clam Project
MIT License
62 stars 58 forks source link

core dump receiving new block on 'reindex' branch #290

Closed dooglus closed 4 years ago

dooglus commented 8 years ago

I'm testing the unreleased 'reindex' branch that xploited was working on. It's crashing occasionally. I caught the most recent crash in gdb:

ProcessBlock() : parent block was previously rejected because of stake duplication. Reaccepting parent
ProcessBlock() : parent block was previously rejected because of stake duplication. Reaccepting parent

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffdef5d700 (LWP 15258)]
CheckStakeKernelHash (pindexPrev=0x0, nBits=453105013, blockFrom=..., nTxPrevOffset=150, txPrev=..., prevout=..., nTimeTx=1459652672, hashProofOfStake=..., 
    targetProofOfStake=..., fPrintProofOfStake=false) at kernel.cpp:401
401     if (IsProtocolV2(pindexPrev->nHeight+1))
(gdb) p pindexPrev
$1 = (CBlockIndex *) 0x0
(gdb) where
#0  CheckStakeKernelHash (pindexPrev=0x0, nBits=453105013, blockFrom=..., nTxPrevOffset=150, txPrev=..., prevout=..., nTimeTx=1459652672, hashProofOfStake=..., 
    targetProofOfStake=..., fPrintProofOfStake=false) at kernel.cpp:401
#1  0x00005555556961e5 in CheckProofOfStake (pindexPrev=pindexPrev@entry=0x0, tx=..., nBits=453105013, hashProofOfStake=..., targetProofOfStake=...)
    at kernel.cpp:436
#2  0x00005555555d3926 in CheckProofOfStake (pindexPrev=pindexPrev@entry=0x0, pblock=pblock@entry=0x7fffc8050b30, hash=..., state=...) at main.cpp:3042
#3  0x00005555555ed538 in ProcessBlock (state=..., pfrom=pfrom@entry=0x7fffd40028b0, pblock=pblock@entry=0x7fffdef5c680, dbp=dbp@entry=0x0) at main.cpp:3178
#4  0x00005555555f1f44 in ProcessMessage (pfrom=pfrom@entry=0x7fffd40028b0, strCommand="block", vRecv=..., nTimeReceived=nTimeReceived@entry=1459652763465474)
    at main.cpp:4631
#5  0x00005555555f2e61 in ProcessMessages (pfrom=0x7fffd40028b0) at main.cpp:4884

The commit I built from was https://github.com/nochowderforyou/clams/commit/fadba934513.

l0rdicon commented 8 years ago

I'm looking into this now, thanks for the debug. Not sure why pindexPrev is null but i will find out

dooglus commented 8 years ago

I notice that the crashing code was replaced by another massive merge of the Bitcoin code in your "Updates to the core files for the changes. To many changes to list" commit (619afaaeaaa) and so may well no longer be a problem.

How well tested is this reindexplus branch?

dooglus commented 8 years ago

Also, I think that pointer was null because I received an orphan block.

ProcessBlock() in main.cpp used to do this until your 619afaae commit:

CBlockIndex* pPrev = NULL;
map<uint256, CBlockIndex*>::iterator mi = mapBlockIndex.find(pblock->hashPrevBlock);
if (mi != mapBlockIndex.end())
    pPrev = (*mi).second;

which would leave pPrev set to NULL if the previous block wasn't in the mapBlockIndex map.

dooglus commented 8 years ago

I got the crash again. It seems to only happen if I've had the client shut down for a while then restart it so it has some 'catching up' to do.

Here's the new crash:

[New Thread 0x7fffcdebb700 (LWP 4175)]
[Thread 0x7fffe721b700 (LWP 4164) exited]
ProcessBlock() : parent block was previously rejected because of stake duplication. Reaccepting parent

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe50d7700 (LWP 4168)]
CheckStakeKernelHash (pindexPrev=0x0, nBits=453041806, blockFrom=..., nTxPrevOffset=150, txPrev=..., prevout=..., nTimeTx=1459780608, hashProofOfStake=..., 
    targetProofOfStake=..., fPrintProofOfStake=false) at kernel.cpp:401
401     if (IsProtocolV2(pindexPrev->nHeight+1))
(gdb) where
#0  CheckStakeKernelHash (pindexPrev=0x0, nBits=453041806, blockFrom=..., nTxPrevOffset=150, txPrev=..., prevout=..., nTimeTx=1459780608, hashProofOfStake=..., 
    targetProofOfStake=..., fPrintProofOfStake=false) at kernel.cpp:401
#1  0x00005555556961e5 in CheckProofOfStake (pindexPrev=pindexPrev@entry=0x0, tx=..., nBits=453041806, hashProofOfStake=..., targetProofOfStake=...)
    at kernel.cpp:436
#2  0x00005555555d3926 in CheckProofOfStake (pindexPrev=pindexPrev@entry=0x0, pblock=pblock@entry=0x7fffd444cf00, hash=..., state=...) at main.cpp:3042
#3  0x00005555555ed538 in ProcessBlock (state=..., pfrom=pfrom@entry=0x7fffd0002bc0, pblock=pblock@entry=0x7fffe50d6680, dbp=dbp@entry=0x0) at main.cpp:3178
#4  0x00005555555f1f44 in ProcessMessage (pfrom=pfrom@entry=0x7fffd0002bc0, strCommand="block", vRecv=..., nTimeReceived=nTimeReceived@entry=1459780631660345)
    at main.cpp:4631
#5  0x00005555555f2e61 in ProcessMessages (pfrom=0x7fffd0002bc0) at main.cpp:4884
dooglus commented 8 years ago

I'll try building your latest doubleplusgood branch or whatever it was called and see how that goes.

haraldg commented 7 years ago

Got the same crash while on the 2.0.0-rc1 branch (during normal operation, no redownloading of the blockchain or similar). I have a debug.log and a back trace but it is pretty much the same as above and in #303 ...

I think the code depending on pPrev not being NULL was introduced in 0c55c7cb6 and a check was added in 1938b48f5, but only around one call of CheckProofOfStake().

I think it is clear where the error is coming from, but I don't know enough about the internals to guess what actually should be done in that case.

haraldg commented 7 years ago

@tryphe After reading your recent comment in the other issue, I'm wondering if you saw my last comment in this issue here? You seem to have more knowledge on clamd internals then I do. Maybe you can come up with the missing bits in my analysis?

tryphe commented 7 years ago

I think you're right, the replacement of this line looks wrong: https://github.com/nochowderforyou/clams/commit/0c55c7cb6fd3accdcb3940db0a3e17ca551ab8c8#diff-7ec3c68a81efff79b6ca22ac1f1eabbaL3035

if (pPrev && pblock->IsProofOfStake()) was replaced with if (!CheckProofOfStake(state, pPrev, pblock, hash) but CheckProofOfStake doesn't sanity check pPrev. That would cause the segfault but I'm not entirely sure of the meaning of it being null.

haraldg commented 7 years ago

That would cause the segfault but I'm not entirely sure of the meaning of it being null.

Yes, that's where I got stuck too. It's easy to add a check for pPrev, but will processing of new blocks get stuck, if we don't add code to handle this case?

l0rdicon commented 4 years ago

Closing issue, unrelated to client v2.1.0