qa: testing for the year 2038 problem #21362

ivanacostarubio · 2021-03-04T17:33:33Z

This is a functional tests to check for the Y2038 issue. See #21356 (comment)

More information: https://en.wikipedia.org/wiki/Year_2038_problem

I'm wondering If this approach is correct. Is so, I can extend it to check for the year 2106 #21356 (comment)

7hacker · 2021-03-04T18:15:33Z

test/functional/y2038.py

+EPOCH_2038_BEFORE = 2147440306 # (GMT): Monday, January 18, 2038 3:11:46 PM
+EPOCH_2038_AFTER = 2147613106 # (GMT): Wednesday, January 20, 2038 3:11:46 PM
+
+class Y2038EpochTest(BitcoinTestFramework):


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.

7hacker

🚢

test/functional/y2038.py

adamjonas · 2021-03-05T15:19:08Z

test/functional/test_runner.py

@@ -201,6 +201,7 @@
    'feature_backwards_compatibility.py --legacy-wallet',
    'feature_backwards_compatibility.py --descriptors',
    'wallet_txn_clone.py --mineblock',
+    'y2038.py',


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.

adamjonas · 2021-03-05T15:24:44Z

Thanks for picking this up @ivanacostarubio

Friendly reminder that these 3 commits should be squashed.

jonatack · 2021-03-05T17:14:32Z

test/functional/test_runner.py

@@ -201,6 +201,7 @@
    'feature_backwards_compatibility.py --legacy-wallet',
    'feature_backwards_compatibility.py --descriptors',
    'wallet_txn_clone.py --mineblock',
+    'example_y2038.py',


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)

test/functional/example_y2038.py

brunoerg · 2021-03-06T23:36:00Z

Tested ACK 00f400b

Ran it on Ubuntu 20.04

nit: I think you could use something like self.log.info on each logic step of the test, because when I executed the test I couldn't follow the logic behind it just looking at the terminal.

test/functional/example_y2038.py

luke-jr · 2021-03-09T18:47:37Z

test/functional/example_y2038.py

+
+EPOCH_2038_BEFORE = 2147440306 # (GMT): Monday, January 18, 2038 3:11:46 PM
+EPOCH_2038_AFTER = 2147613106 # (GMT): Wednesday, January 20, 2038 3:11:46 PM
+EPOCH_2106 = 4293404400000


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

luke-jr · 2021-03-09T18:47:53Z

test/functional/example_y2038.py

+            self.nodes[1].setmocktime(epoch)
+
+            # Generates a blocks
+            self.nodes[0].generate(nblocks=1)


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:

bitcoin/src/primitives/block.h

Line 27 in 4a54068

uint32_t nTime;

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)

leonardojobim

The tests cedfabf using EPOCH_2106 = 4293404400 ran with no issues on Ubuntu 20.04.

NelsonGaldeman

I agree with jon the example prefix may not be the best fit. I would suggest p2p as you are actually testing syncing blocks between 2 nodes.

tACK 00f400b on OSX 11.4

NelsonGaldeman · 2021-07-16T16:38:57Z

test/functional/example_y2038.py

+EPOCH_2106 = 4293404400000
+
+
+class Y2038EpochTest(BitcoinTestFramework):


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

theStack

Concept ACK

Thanks for adding tests! Could you squash the three commits? Also I agree with previous comments that the naming of the test file could be improved, example_... doesn't seem appropriate.

DrahtBot · 2022-01-11T12:11:47Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#20892 (tests: Run both descriptor and legacy tests within a single test invocation by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2022-02-22T11:51:51Z

Needs rebase due to silent merge conflict

maflcko

(left a comment with the patch needed to fix the conflict)

maflcko · 2022-02-22T11:53:41Z

test/functional/example_y2038.py

+            self.nodes[1].setmocktime(epoch)
+
+            # Generates a blocks
+            self.nodes[0].generate(nblocks=1)


Suggested change

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

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

fanquake · 2022-04-26T15:04:29Z

@ivanacostarubio Did you want to follow up here?

fanquake · 2022-10-02T14:45:56Z

No follow up, so marking up for grabs, and closing for now. Leave a comment if you'd like this reopened etc.

fafc96a test: Test year 2106 block timestamps (MacroFake) Pull request description: Alternative to bitcoin/bitcoin#21362 that closes bitcoin/bitcoin#21356 ACKs for top commit: Sjors: utACK fafc96a Tree-SHA512: 196d98f42d6f7f0222312b7bd1c68b3bd30cb6f0cbaccb900cfc5fcc689494adb2a7d7d6023c1ff1e8cf871047ec37eeca41386e31029d99cabf9343b4fd2a03

fafc96a test: Test year 2106 block timestamps (MacroFake) Pull request description: Alternative to bitcoin#21362 that closes bitcoin#21356 ACKs for top commit: Sjors: utACK fafc96a Tree-SHA512: 196d98f42d6f7f0222312b7bd1c68b3bd30cb6f0cbaccb900cfc5fcc689494adb2a7d7d6023c1ff1e8cf871047ec37eeca41386e31029d99cabf9343b4fd2a03

laanwj added the Tests label Mar 4, 2021

7hacker suggested changes Mar 4, 2021

View reviewed changes

7hacker approved these changes Mar 5, 2021

View reviewed changes

adamjonas reviewed Mar 5, 2021

View reviewed changes

test/functional/y2038.py Outdated Show resolved Hide resolved

adamjonas reviewed Mar 5, 2021

View reviewed changes

ivanacostarubio force-pushed the y2038_tests branch 2 times, most recently from 3217632 to 353aa84 Compare March 5, 2021 16:03

qa: testing for the year 2038 problem

00f400b

ivanacostarubio force-pushed the y2038_tests branch from 353aa84 to 00f400b Compare March 5, 2021 17:01

jonatack reviewed Mar 5, 2021

View reviewed changes

test/functional/example_y2038.py Outdated Show resolved Hide resolved

brunoerg reviewed Mar 6, 2021

View reviewed changes

test/functional/example_y2038.py Outdated Show resolved Hide resolved

feedback on github

5a5c330

brunoerg reviewed Mar 8, 2021

View reviewed changes

test/functional/example_y2038.py Outdated Show resolved Hide resolved

debuging should have been removed

cedfabf

luke-jr reviewed Mar 9, 2021

View reviewed changes

leonardojobim reviewed Mar 10, 2021

View reviewed changes

NelsonGaldeman reviewed Jul 16, 2021

View reviewed changes

theStack reviewed Dec 20, 2021

View reviewed changes

DrahtBot mentioned this pull request Jan 11, 2022

tests: Run both descriptor and legacy tests within a single test invocation #20892

Closed

maflcko reviewed Feb 22, 2022

View reviewed changes

fanquake added the Up for grabs label Oct 2, 2022

fanquake closed this Oct 2, 2022

maflcko mentioned this pull request Oct 5, 2022

test: Test year 2106 block timestamps #26259

Merged

maflcko removed the Up for grabs label Oct 5, 2022

bitcoin locked and limited conversation to collaborators Oct 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

qa: testing for the year 2038 problem #21362

qa: testing for the year 2038 problem #21362

ivanacostarubio commented Mar 4, 2021

7hacker Mar 4, 2021

IamLupo Mar 4, 2021 •

edited

Loading

ivanacostarubio Mar 5, 2021

jonatack Mar 5, 2021

ivanacostarubio Mar 8, 2021

7hacker left a comment

adamjonas Mar 5, 2021

ivanacostarubio Mar 5, 2021

adamjonas commented Mar 5, 2021

jonatack Mar 5, 2021

ivanacostarubio Mar 8, 2021

brunoerg commented Mar 6, 2021

luke-jr Mar 9, 2021

ivanacostarubio Mar 10, 2021

luke-jr Mar 9, 2021

IamLupo Mar 9, 2021 •

edited

Loading

luke-jr Mar 9, 2021

IamLupo Mar 9, 2021 •

edited

Loading

ivanacostarubio Mar 10, 2021

leonardojobim left a comment •

edited

Loading

NelsonGaldeman left a comment

NelsonGaldeman Jul 16, 2021

theStack left a comment

DrahtBot commented Jan 11, 2022 •

edited

Loading

maflcko commented Feb 22, 2022

maflcko left a comment

maflcko Feb 22, 2022

fanquake commented Apr 26, 2022

fanquake commented Oct 2, 2022

		EPOCH_2106 = 4293404400000


		class Y2038EpochTest(BitcoinTestFramework):

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

qa: testing for the year 2038 problem #21362

qa: testing for the year 2038 problem #21362

Conversation

ivanacostarubio commented Mar 4, 2021

Choose a reason for hiding this comment

IamLupo Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

7hacker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamjonas commented Mar 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunoerg commented Mar 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IamLupo Mar 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IamLupo Mar 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonardojobim left a comment • edited Loading

Choose a reason for hiding this comment

NelsonGaldeman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theStack left a comment

Choose a reason for hiding this comment

DrahtBot commented Jan 11, 2022 • edited Loading

Conflicts

maflcko commented Feb 22, 2022

maflcko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fanquake commented Apr 26, 2022

fanquake commented Oct 2, 2022

IamLupo Mar 4, 2021 •

edited

Loading

IamLupo Mar 9, 2021 •

edited

Loading

IamLupo Mar 9, 2021 •

edited

Loading

leonardojobim left a comment •

edited

Loading

DrahtBot commented Jan 11, 2022 •

edited

Loading