CCoinsKeyHasher::operator() should return size_t #4635

It currently returns uint64_t, which on older boost (at least 1.46) causes failures on 32-bit systems. This problem was introduced in bc42503.

Fixes #4634.

Any chance of a regression test added to the test suite?

@jgarzik The tests already fail without this, on the affected environments.

Right, the tests already failed. But no one has been running them with that version of boost, at least on a 32-bit system.

The Boost version used by the pulltester is even older than 1.46, and it passed:

#define BOOST_LIB_VERSION "1_40"

It's also fine in 1.54+. Seemingly the problem with non-size_t-hashers only exists in a relatively small range of boost versions.

Not sure I understand, but key.GetHash(salt) gives a uint64_t and we now cast that to size_t? Shouldn't some more places be size_t then and not just this changed return value?

@diapolo GetHash() is a 64-bit hash function so it will always give a 64-bit result. Casting it to size_t is just to make boost happy. It may chop off the upper 32 bits on systems where size_t is 32 bits, but for a well-designed hash function that doesn't matter as the properties stay the same.

@laanwj I would feel better if you could add a small comment about that and why it was changed.

Will add a comment.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4635_6c23b082033b627f31170166c07ab35fa6be9343/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

ACK, thanks for adding that comment.

@sipa Since std::unordered_map is now used instead of boost::unordered_map, would it be beneficial to revert SaltedOutpointHasher::operator() to return uint_64t?