What would be the downside of a forced 64bit type on 32bit platform? I guess the performance reduction and the slightly higher memory consumption is totally negligible?

Would fixing it "on the other side" (detect size_t overflow while hashing, force the 32bit reset on 64bit platforms) not involve less risks?

@laanwj Nice catches. utACK.

@jonasschnelli I think it's not worth the trouble. For platforms constrained enough for the change to be significant, I suspect that a separate hashing implementation would've already been patched in.

@sipa I know you have bigger plans for this. Do you disagree, though, that this is an improvement? I'm using gettxoutsetinfo a lot so it would really help me if this is fixed.

@jonasschnelli I'd say doing the hashing as it is supposed to be done, with counters of the appropriate width - as done here - is less risky than trying to hack something with overflow detection? Adding a 64-bit with 32-bit value is cheap on 32-bit architectures as well, it even involves overflow detection (the carry flag) internally.

The hash function code should be correct with large inputs regardless of what the utxoset hash stuff returns. utACK.

@laanwj Oops, I just responded to @jonasschnelli's comment, not seeing there was a patch too.

Yes, uint64_t should definitely be used for at least SHA1, SHA256 and RIPEMD160. In theory, we'd need to use a 128-bit integer for SHA512 (as it writes a 128-bit length descriptor in the padding), but I guess we'll never hit that.

there can be an assert to catch the overflow in the sha512 case, if we want to be pedantic?

In theory, we'd need to use a 128-bit integer for SHA512 (as it writes a 128-bit length descriptor in the padding), but I guess we'll never hit that.

Good catch!

Although implementing actual 128-bit counting is just as much work as adding an overflow assertion, I'd personally say adding an assert is preferable here, as we can never test this behavior.

The assert should probably exist for SHA1/256/RIPEMD160 as well, as those are undefined for longer messages.

Hmm looking at the actual code I'll leave that for another time. At least this is trivially correct. I feel that the risk of someone hashing 18,446,744 TB of data is extremely low in the foreseeable future (even with the unfortunate rate of UTXO growth that will take a while), at least smaller than the risk of me introducing bugs there.

utACK 088c270c2928089f1903db0ff01c82eaaa5f6e45

Concept ACK

Tested ACK, on an OSX 64-bit machine.

On a debug build on my machine, hashing the txids makes gettxoutsetinfo about ten seconds slower (3 minutes versus 2 minutes 50 seconds).