Weaken serfloat tests

[sipa]

Closes #28941.

Our current tests for serfloat verify two distinct properties:

1. Whether they roundtrip double->uint64_t->double (excluding NaN values) on all systems.
2. Whether on systems with a typical floating point unit that encoding matches the hardware representation, as before v22.0, we would dump the hardware representation directly to disk and we wanted to retain compatibility with that.

#28941 seems to show that the second property doesn't always hold, but just for "subnormal" numbers (below $2^{-1021}$). Since we don't care about encoding these numbers, we could exclude such subnormal numbers from the hardware-identical representation test, but this PR goes further and just drops the second property entirely, as I don't think we care about edge-case compatibility with pre-v22.0 code for fee_estimates.dat (the only place it is used).

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	glozow, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

[maflcko]

I think the fuzz tests are also checking the exact hardware representation? So they should be failing as well, but I guess no one is running them through homebrew (or whatever the affected system is).

[sipa]

@maflcko I don't think they are affected, as the fuzz tests always start from a double (even if one obtained by decoding raw memory) and test roundtripping double->uint64_t->double of that. The issue only seems to appear when starting from raw memory, then encoding + decoding, and comparing with the raw memory started with.

[sipa]

@maflcko I've added a commit to verify that... it adds a hardware-representation equivalence test again, but starting from a double like the fuzz tests do.

[fanquake]

Pushed up the patches into my PR, it has fixed some of the test failures, but not all: https://github.com/Homebrew/homebrew-core/actions/runs/7446626418/job/20257419440?pr=156745#step:4:82.

  ==> /home/linuxbrew/.linuxbrew/Cellar/bitcoin/26.0/bin/test_bitcoin
  Running 585 test cases...
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [1 != 0]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9223372036854775809 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9227078312698333629 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9227541302258710164 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9226320440948298264 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9224296035333843957 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [3342972290171876 != 0]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [455962168228056 != 0]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9226157041067996541 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9225839089286856993 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9225901867614656042 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9227430019473490172 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9224285440633669494 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9224305768567079357 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9223409755742115848 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9224058209782467602 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [2599738216344523 != 0]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9227049248406766111 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9226155215443275398 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [4045757423585278 != 0]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9225916179490235579 != 9223372036854775808]
  test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [9227295753411824867 != 9223372036854775808]
  
  *** 22 failures are detected in the test module "Bitcoin Core Test Suite"

If anyone wants to reproduce the failures locally (against master), they can do:

docker pull docker.io/homebrew/brew:latest
docker run -it homebrew/brew

brew install bitcoin --head
brew test bitcoin

==> Testing bitcoin
==> /home/linuxbrew/.linuxbrew/Cellar/bitcoin/HEAD-04b9df0/bin/test_bitcoin
Last 15 lines from /home/linuxbrew/.cache/Homebrew/Logs/bitcoin/test.01.test_bitcoin:
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [602650259075964 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [9225028085348500152 != 9223372036854775808]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [225556757220273 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [808287137814849 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [1601366725718274 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [1803809784509295 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [9224578429555196467 != 9223372036854775808]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [2210598722220679 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [869264799447506 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [1193579134805228 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [1018605001172173 != 0]
test/serfloat_tests.cpp(96): error: in "serfloat_tests/double_serfloat_tests": check v == v2 has failed [9226505921543298662 != 9223372036854775808]

*** 219 failures are detected in the test module "Bitcoin Core Test Suite"

[sipa]

@fanquake What if you drop the last commit here?

[fanquake]

What if you drop the last commit here?

Tests are passing: https://github.com/Homebrew/homebrew-core/actions/runs/7448770957/job/20264045705?pr=156745.

[sipa]

Dropped the last commit, and improved the code changes a bit. For backport, if needed, the first commit suffices.

[luke-jr]

Does dropping 2 lose the test that we haven't broken the serialization format somehow? Maybe it should be comparing a fixed mapping of some values at least?

[glozow]

ACK ea60c95be36a68ba98d1d23018587aa4d4d6bb1a, based on https://github.com/Homebrew/homebrew-core/pull/156745 and https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1881144857 it looks like this fixes the issue.

re https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1906925728, the values that were failing the tests are irrelevant in the context we're using this, and 21.0 has been eol for a while now - also see https://github.com/bitcoin/bitcoin/issues/28941#issuecomment-1879103731

second commit diff looks correct to me though I'm unsure if it's necessary (ACK e5202418cbec34ffd0269ce66293d9c41b20709b)

[sipa]

@luke-jr There were a few expected encoding value tests, but I've expanded them significantly now.

[sipa]

Anything left to do here?

[fanquake]

Backported to 27.x in #29693.