[ironfish-rust]Duplicate Randomnesses are calculated

[hairtail]

What happened?

Source code here, mine_batch. Example:

thread_count = 32
step_size = 32
batch_size = 100
search space of each threads should be:
1st: (0, 100), (100, 200)...  
2nd: (1, 101), (101, 201)...
...
32th: (31, 131), (131, 231)...
Randomnesses during first round for each threads should be:
1st: [0, 32, 64, 96]
2nd: [1, 33, 65, 97]
4th: [4, 36, 68, 100]
5th: [5, 37, 69, 101]
Randomnesses during second round for each threads should be:
1st: [100, 132, 164, 196]
2nd: [101, 133, 165, 197]
[100, 101, ...] are calculated twice.

Version

0.1.46 @ src

Debug output

No response

Relevant log output

No response

Graffiti

miningKing

[mat-if]

Thanks for reporting this - definitely a bug. https://github.com/iron-fish/ironfish/pull/2187 should fix it

[hairtail]

With #2187, the example above still exists.

[scuwan]

Perhaps it's better to split the search space this way: subspace_size = u64::MAX/thread_count; [subspace_size * 0.. subspace_size*1)... [subspace_size * id.. subspace_size * (id +1)]... [subspace_size * (thread_count - 1)..subspace_size * thread_count)

[mat-if]

With #2187, the example above still exists.

~~I just did some more testing, and I don't think it was.~~ I think it was minimized with that first change, but it was likely happening a bit still. By default, the range is not inclusive, so while it would show an end that overlapped, that number would never get executed in the first loop. However, I've since refactored it to make it more clear by making it inclusive and a smaller end reduced by step_size.

[mat-if]

Perhaps it's better to split the search space this way: subspace_size = u64::MAX/thread_count; [subspace_size * 0.. subspace_size*1)... [subspace_size * id.. subspace_size * (id +1)]... [subspace_size * (thread_count - 1)..subspace_size * thread_count)

While simpler at this level, that creates a hard loop that will never stop to check if it's received a new command. It's probably possible to accomplish the same task, but it would lead to more complex code I think since you now have to figure out a way to manage thread state from the main thread

[scuwan]

I found this bug because my mine pool was receiving duplicate shares from miners.

[scuwan]

Perhaps it's better to split the search space this way: subspace_size = u64::MAX/thread_count; [subspace_size * 0.. subspace_size*1)... [subspace_size * id.. subspace_size * (id +1)]... [subspace_size * (thread_count - 1)..subspace_size * thread_count)

While simpler at this level, that creates a hard loop that will never stop to check if it's received a new command. It's probably possible to accomplish the same task, but it would lead to more complex code I think since you now have to figure out a way to manage thread state from the main thread

I can give you the implementation if you want

[scuwan]

A simple solution would be to make batch_size an integer multiple of thread_count, which I think would work and preserve the logic