Quote from: Nite69 on April 05, 2014, 01:28:55 PM

Quote from: vilgem on April 05, 2014, 10:21:53 AM

I looked at the diff file you proposed very thoroughly. And I must conclude that your fix is nothing more but a bullshit. The only thing your fix does - it protects PastRateActualSeconds varibale in a way that it is always >= 1 (second). Old code assumed it was always >= 0. You probably cared about PastRateAdjustmentRatio variable which is equal to 1 if PastRateActualSeconds happens to be 0. But the point is that it never happens. KGW takes at least PastBlocksMin and at most PastBlocksMax blocks into the calculation. You will never have PastRateActualSeconds == 0 except for the case if your blockchain has only one block.

You have misunderstood the fix. There are 2 fixes and the one you are referring fixes another attack vector.

The fix to usual TW attack (which BCX was planning was to use) was to use LatestBlockTime instead of BlockLastSolved->GetBlockTime() to count the timespan. Without this one can timetravel back without diff rise, with this the benefit attacker gets by travellin past is lost.

The another fix (preventing PastRateActualSeconds to go to 0) takes care of another attack vector. Here is a short explanation of the attack:
1. generate a block 2 weeks to the future. You cannot publish it, it is not on current time window.
2. Start generating blocks with the same timestamp (ie the moment 2 weeks in the future)

See what would happen: after there is PastBlocksMax blocks in the private chain, *the diff would not change* at all!

That would mean you have 2 weeks to generate blocks with 0 difficulty. With decent hashrate, you easily get 1 block in a second. In 2 weeks you get 1209600 blocks.

When that 2 weeks has passed, what would happen to the blockchain, if you suddenly publish 1209600 perfectly valid blocks? The whole network would be doing nothing but checking those 1209600 blocks... and finding nothing wrong with them. That would be the end of the coin.

Quote

You will never have PastRateActualSeconds == 0 except for the case if your blockchain has only one block.

Thats not true. You can generate blocks with the same timestamp. Or is there something that would prevent it (I have not read all the code, it might be prevented somewhere) ? If there is, then this attack vector was already closed and this part was not necessary.

EDIT: Actually, it *is* prevented somewhere else. One can generate only 5 blocks with the same timestamp. So this #2 fix is not necessary to prevent that attack vector, it is already closed elsewhere. However that means the whole if clauses are never true, ie they are itself worthless. But leaving them as they were would keep there an unecessary dependancy between the code blocks, so it is nevertheless better to change it. Also, the main fix is #1, which has been confirmed to work.

Code:

// Check timestamp against prev
if (GetBlockTime() <= pindexPrev->GetMedianTimePast())
return error("AcceptBlock() : block's timestamp is too early");

Ok, it turned out that you can't publish a block with a timestamp less or equal to the median time of several prior blocks. So, fix #2 is not necessary. Even if you could the attack you mentioned would be very unlikely. One would have to start working on a brach chain of PastBlocksMax blocks of the same timestamp. With every new block found the actual difficulty would RAISE (timespan would diminish with every new block found). It would be an exponential difficulty growth for several thousands iterations (the base difficulty would be equal to the network difficulty at the fork time). So, one would have to possess really good hashing power. I think that mathematically this attack is even less possible than wellknown 51% attack. More precisely this attack would have possibility 100% but the computation time would be HUGE.

Now... fix #1. I'm sorry but is not a fix. Logically both flows (without fix and with it) are absolutelly THE SAME. Just check it line by line very carefuly. Both flows protect PastRateActualSeconds variable in a way that it is >= 0. That is it. Just CHECK.