tests: fix flaky abc_rpc_mocktime test and limit max mocktime in RPC
AbandonedPublic
Actions

Authored by PiRK on Apr 9 2024, 09:03.

Details

Reviewers

None

Group Reviewers

Restricted Project

Summary

After D14912, the abc_rpc_mocktime test occasionnaly triggers an assertion error in time.cpp on slow machines

AssertionError: Unexpected stderr bitcoind: /home/pierre/dev/bitcoin-abc/src/util/time.cpp:78: static NodeClock::time_point NodeClock::now(): Assertion `ret > 0s' failed. !=

This is because NodeClock::Now() returns a time with nanoseconds precision (encoded as an int64 internally), no matter if the time is mocked or not, and the test tries to set a mocktime of max_int64 seconds.

Fix the RPC to not accept values that would cause an overflow.

Test Plan

Add import time; time.sleep(2] after setmocktime in the abc_rpc_mocktime test to ensure the NodeClock::now() assertion is triggered in case of overflow.
Verify that before this change the test node crashes, with max_int64 // 10**9 + 1 it also crashed, and with max_int64 // 10**9 it does not crash.

After this change, the RPC now limits the max value.

ninja check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Branch

fix_mocktime_test

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 28412
Build 56365: Build Diff	build-clang · build-clang-tidy · build-without-wallet · build-debug · build-diff
Build 56364: arc lint + arc unit

Event Timeline

PiRK created this revision.Apr 9 2024, 09:03

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 9 2024, 09:03

PiRK requested review of this revision.Apr 9 2024, 09:03

PiRK edited the test plan for this revision. (Show Details)Apr 9 2024, 09:05

Let's have the RPC return an error also, to guard against future flaky tests caused by overflows.

Harbormaster completed remote builds in B28412: Diff 46955.Apr 9 2024, 09:13

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[447/492] Running utility command for check-pow-grasberg_tests
[448/492] Running pow test suite
PASSED: pow test suite
[449/492] bitcoin: testing sighash_tests
[450/492] bitcoin: testing blockmanager_tests
[451/492] bitcoin: testing rcu_tests
[452/492] Running utility command for check-bitcoin-sighash_tests
[453/492] bitcoin: testing validation_chainstate_tests
[454/492] Running utility command for check-bitcoin-blockmanager_tests
[455/492] Running utility command for check-bitcoin-rcu_tests
[456/492] Running utility command for check-bitcoin-validation_chainstate_tests
[457/492] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[458/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[459/492] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.011s

OK
[460/492] bitcoin: testing wallet_crypto_tests
[461/492] Running utility command for check-bitcoin-wallet_crypto_tests
[462/492] bitcoin: testing blockcheck_tests
[463/492] Running utility command for check-bitcoin-blockcheck_tests
[464/492] bitcoin: testing coinselector_tests
[465/492] Running utility command for check-bitcoin-coinselector_tests
[466/492] bitcoin: testing wallet_tests
[467/492] Running utility command for check-bitcoin-wallet_tests
[468/492] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[469/492] Linking CXX executable src/seeder/test/test-seeder
[470/492] seeder: testing message_writer_tests
[471/492] seeder: testing options_tests
[472/492] seeder: testing p2p_messaging_tests
[473/492] seeder: testing parse_name_tests
[474/492] Running utility command for check-seeder-p2p_messaging_tests
[475/492] Running utility command for check-seeder-message_writer_tests
[476/492] Running utility command for check-seeder-options_tests
[477/492] Running utility command for check-seeder-parse_name_tests
[478/492] seeder: testing write_name_tests
[479/492] Running utility command for check-seeder-write_name_tests
[480/492] Running seeder test suite
PASSED: seeder test suite
[481/492] bitcoin: testing coins_tests
[482/492] Running utility command for check-bitcoin-coins_tests
[483/492] Running bitcoin test suite
PASSED: bitcoin test suite
[484/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[485/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[486/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[487/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[488/492] Linking CXX executable src/qt/test/test_bitcoin-qt
[489/492] bitcoin-qt: testing test_bitcoin-qt
[490/492] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

add a check for the upper limit in the RPC command, test it

PiRK edited the summary of this revision. (Show Details)Apr 9 2024, 09:25

Harbormaster completed remote builds in B28413: Diff 46956.Apr 9 2024, 09:47

This would work only for my linux machine. Ideally the max value should be computed for each system ( std::chrono::system_clock::now().max() * std::chrono::system_clock::period), but that is overkill for a test RPC.

PiRK mentioned this in D15927: tests: remove flaky mocktime test.Apr 9 2024, 10:39

PiRK mentioned this in rABCb132af74fbce: tests: remove flaky mocktime test.Apr 9 2024, 12:08