Skip to content

Commit

Permalink
fix-test for the tree-hash bug (abort on bug)
Browse files Browse the repository at this point in the history
  • Loading branch information
rfree2monero committed Sep 4, 2014
1 parent 1d6372c commit b417abf
Showing 1 changed file with 58 additions and 6 deletions.
64 changes: 58 additions & 6 deletions src/crypto/tree-hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,87 @@
#include <assert.h>
#include <stddef.h>
#include <string.h>
#include <stdio.h>

#include "hash-ops.h"

// HASH_SIZE = 32
void tree_hash(const char (*hashes)[HASH_SIZE], size_t count, char *root_hash) {
assert(count > 0);
assert(count > 0); // old code
size_t size_hashes = count;

if (count >= 500) printf("\n================================================= bigblock \n");
printf("tree_hash() count=%zu \n", count);

assert(root_hash!=NULL);
assert(hashes!=NULL);

if (count == 1) {
memcpy(root_hash, hashes, HASH_SIZE);
memcpy(root_hash, hashes, HASH_SIZE); // ok?
} else if (count == 2) {
cn_fast_hash(hashes, 2 * HASH_SIZE, root_hash);
cn_fast_hash(hashes, 2 * HASH_SIZE, root_hash); // ok?
} else {
size_t i, j;

// orginal way ----
size_t cnt = count - 1;
char (*ints)[HASH_SIZE];
for (i = 1; i < sizeof(size_t); i <<= 1) {
cnt |= cnt >> i;
}
cnt &= ~(cnt >> 1);
ints = alloca(cnt * HASH_SIZE);
memcpy(ints, hashes, (2 * cnt - count) * HASH_SIZE);
// cnt is the result

{
// TODO use correct type
int cnt2 = -1;
int tmp = count -1; //using input minus 1 to get fun(2**n)= 2**(n-1)
for (int j = 1; tmp != 0;++j) {
tmp/=2; //dividing by 2 until to get how many powers of 2 fits into tmp
// tmp >>= 1; //will do as well
cnt2 = 2<<(j-2);
//if(tmp == 0) printf("%d: %d\n",i, 2<<(j-2)); // returning i and result of rounding
}
if (cnt != cnt2) {
printf("wrong counter: cnt=%zu versus correct cnt2=%d.", cnt, cnt2);
}
assert( cnt == cnt2 );
}


const size_t size_ints = cnt * HASH_SIZE; // dbg
assert( size_ints > 0); assert( size_ints < 1024*1024 ); // dbg
ints = alloca(size_ints); // ok
assert(ints!=NULL);

const size_t size_memcpy = (2 * cnt - count) * HASH_SIZE;
printf("calculate size_memcpy: cnt=%zu count=%zu size_memcpy=%zu \n" , cnt,count,size_memcpy);
assert( size_memcpy >= 0); // dbg, allow 0 ?
assert( size_memcpy < 1024*1024); // dbg

assert( size_memcpy <= size_ints); // dbg - test for memcpy() ???? <
// assert( size_memcpy <= HASH_SIZE); // dbg - test for memcpy() ???? <
memcpy(ints, hashes, size_memcpy); // ok?

for (i = 2 * cnt - count, j = 2 * cnt - count; j < cnt; i += 2, ++j) {
assert( i < size_hashes ); // hashes[] <<<< haha
// assert( i < count );
assert( j < size_ints ); // ints[]
// assert( j < count );
cn_fast_hash(hashes[i], 64, ints[j]);
}
assert(i == count);

assert(i == count); // dbg

while (cnt > 2) {
cnt >>= 1;
for (i = 0, j = 0; j < cnt; i += 2, ++j) {
assert( i < size_ints ); // ints[i]
assert( j < size_ints ); // ints[j]
cn_fast_hash(ints[i], 64, ints[j]);
}
}

cn_fast_hash(ints[0], 64, root_hash);
}
}

3 comments on commit b417abf

@rfree2monero
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use this one, just for research real PR will follow

@rfree2monero
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit is the first commit as a quick abort and research, CN released information half hour later.

Our full nice solution with proper asserts and documentation on the problem, is here:
https://github.com/rfree2monero/bitmonero/tree/pr-fix-treehash2

@rfree2monero
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap this version indeed was not good, it does first 2 << -1 operation, the calculation result is replaced with correct one always, but still no program should do that.

As I said, the real approved fix I prepared in the other branch, and only after review it goes into the Monero official client.

Please sign in to comment.