qa: testing for the year 2038 problem #21362

A similar test for 2106 to handle an unsigned 32-bit time_t type. See https://en.bitcoin.it/wiki/Block_timestamp

Wouldn't it be a better test that you set the time on the machine itself and let it test what value std::chrono::system_clock will generate? This way you can accually test when std::chrono is been updated and what the epoch value accually be and how much bits it uses.

Yes. I'm going to take a stab at it.

Depending on the approach you end up using, it might be an interesting exercise to try writing this as one of the C++ unit tests, once you have it figured out in Python.

I'll work on the unit test. It will be interesting.

Thank you for the review @jonatack.

trailing whitespace here is breaking the linter

This naming convention doesn't work. Check out https://github.com/bitcoin/bitcoin/blob/a9335e4f129cadd87fda0fe49baf74670ded491a/test/functional/test_runner.py#L690 to see a list of accepted prefixes.

Got it. Thanks. I've prefixed the name with example_. However, there is only one there and this tells me the prefix should be something else.

I'm also wondering if this is the right approach to test this as the tests passes.

the file perms should be 755 (sudo chmod 755 test/functional/example_y2038.py)

-rwxr-xr-x 1   9070 Mar  1 22:43 example_test.py*
-rw-r--r-- 1   1301 Mar  5 18:06 example_y2038.py

If this is intended to be a real test, ISTM it should have another file name.

Indeed. What would you name a test like this @jonatack assuming this approach is correct ? (unit test may be better)

Per PEP8 this import should be at the top of the imports. But it does not seem needed.

Got it. Thanks for pointing this out. Removed.

You can remove this import since you're not using the lib, probably lint got it.

Yes. Thank you.

            # Generates one block

I think 'one block' would be better since you're generating just one block.

Yes. Thanks you Bruno.

This is AD 138022, not AD 2038.

You are correct. Thank you for pointing this out.

ƛ date -r 4293404400000 '+%m/%d/%Y'
07/27/138022

I'm going to change it to: 4293404400 to test for 2106. See the last sentence on: https://en.bitcoin.it/wiki/Block_timestamp

date -r 4293404400 '+%m/%d/%Y'
01/19/2106

Note that the expected behaviour for AD 2106 and beyond is for all blocks to be invalid... This should fail.

yeah thats because there isnt not enough bits difined at specific parts of the code. It needs to be changed to uint64_t as example: https://github.com/bitcoin/bitcoin/blob/4a540683ec40393d6369da1a9e02e45614db936d/src/primitives/block.h#L27

Changing the type there is a hardfork. Also not a good approach (makes more sense to just allow it to wrap around).

Defining a hardfork goes beyond writing tests. A test should simply confirm such blocks are invalid.

Changing the type there is a hardfork. Also not a good approach (makes more sense to just allow it to wrap around).

Defining a hardfork goes beyond writing tests. A test should simply confirm such blocks are invalid.

Well a approach could be that you first implement this type change in a patch and distribute it but just not activate it yet. You just set a block-id when this update should be active. That its enough time to let the hole network been updated. You basicly keep using for the old blocks the old uint32_t type. But new blocks will be uint64_t after that.

Only requirement is that enough nodes in the network has to get the latest updated. Because this bug has a long time to trigger you can delay to active this feature/fix for really long time. Like you know that every node has the update over the next view years (say 2030). Also you have a lot of time to distribute this knowledge to the community like this.

Got it. I'm going to change the test to reflect that. I also saw it failed at one of our CI checks (https://cirrus-ci.com/task/6103755461492736?command=ci#L5253), but not on the others https://cirrus-ci.com/task/4626011833761792?command=ci#L3610

 node0 stderr miner.cpp:130:21: runtime error: implicit conversion from type 'int64_t' (aka 'long') of value 4293404400000 (64-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 2732071296 (32-bit, unsigned)

            self.generate(self.nodes[0], nblocks=1)

nit: All the naming (class, method, logs, file name) reference the year 2038 but it tests the year 2106 as well!