JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
731 stars 179 forks source link

Fidelity bond wallet tests are failing on 32-bit archs #798

Closed kristapsk closed 3 years ago

kristapsk commented 3 years ago

Noticed while testing #796 on armv7l (although modern Raspberry Pi's are technically 64-bit boards, default Linux used there is still 32-bit). Guess we could just remove locktimes after year 2038 there? Or we should add support for years above 2038 on archs where time_t is 32-bit? Ping @chris-belcher.

________________________________________________________________________ test_gettimelockaddress_method[350-2049-03] _________________________________________________________________________

setup_wallet = None, timenumber = 350, locktime_string = '2049-03'

    @pytest.mark.parametrize('timenumber,locktime_string', [
        [0, "2020-01"],
        [20, "2021-09"],
        [100, "2028-05"],
        [150, "2032-07"],
        [350, "2049-03"]
    ])
    def test_gettimelockaddress_method(setup_wallet, timenumber, locktime_string):
        storage = VolatileStorage()
        SegwitLegacyWalletFidelityBonds.initialize(storage, get_network())
        wallet = SegwitLegacyWalletFidelityBonds(storage)

        m = FidelityBondMixin.FIDELITY_BOND_MIXDEPTH
        address_type = FidelityBondMixin.BIP32_TIMELOCK_ID
        index = wallet.get_next_unused_index(m, address_type)
        script = wallet.get_script_and_update_map(m, address_type, index,
            timenumber)
        addr = wallet.script_to_addr(script)

>       addr_from_method = wallet_gettimelockaddress(wallet, locktime_string)

jmclient/test/test_wallet.py:287: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
jmclient/jmclient/wallet_utils.py:1186: in wallet_gettimelockaddress
    lock_datetime.timetuple()))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'jmclient.wallet.FidelityBondMixin'>, timestamp = 2498169600

    @classmethod
    def timestamp_to_time_number(cls, timestamp):
        """
        converts a datetime object to a time number
        """
>       dt = datetime.utcfromtimestamp(timestamp)
E       OverflowError: timestamp out of range for platform time_t

jmclient/jmclient/wallet.py:2162: OverflowError
________________________________________________________________________ test_timestamp_to_timenumber[2366841600-300] ________________________________________________________________________

setup_wallet = None, timestamp = 2366841600, timenumber = 300

    @pytest.mark.parametrize('timestamp,timenumber', [
        [1577836800, 0],
        [1709251200, 50],
        [2366841600, 300],
        [1577836801, None], #not exactly midnight on first of month
        [2629670400, None], #too far in future
        [1575158400, None] #before epoch
    ])
    def test_timestamp_to_timenumber(setup_wallet, timestamp, timenumber):
        try:
            implied_timenumber = FidelityBondMixin.timestamp_to_time_number(
>               timestamp)

jmclient/test/test_wallet.py:652: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'jmclient.wallet.FidelityBondMixin'>, timestamp = 2366841600

    @classmethod
    def timestamp_to_time_number(cls, timestamp):
        """
        converts a datetime object to a time number
        """
>       dt = datetime.utcfromtimestamp(timestamp)
E       OverflowError: timestamp out of range for platform time_t

jmclient/jmclient/wallet.py:2162: OverflowError
_______________________________________________________________________ test_timestamp_to_timenumber[2629670400-None] ________________________________________________________________________

setup_wallet = None, timestamp = 2629670400, timenumber = None

    @pytest.mark.parametrize('timestamp,timenumber', [
        [1577836800, 0],
        [1709251200, 50],
        [2366841600, 300],
        [1577836801, None], #not exactly midnight on first of month
        [2629670400, None], #too far in future
        [1575158400, None] #before epoch
    ])
    def test_timestamp_to_timenumber(setup_wallet, timestamp, timenumber):
        try:
            implied_timenumber = FidelityBondMixin.timestamp_to_time_number(
>               timestamp)

jmclient/test/test_wallet.py:652: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

cls = <class 'jmclient.wallet.FidelityBondMixin'>, timestamp = 2629670400

    @classmethod
    def timestamp_to_time_number(cls, timestamp):
        """
        converts a datetime object to a time number
        """
>       dt = datetime.utcfromtimestamp(timestamp)
E       OverflowError: timestamp out of range for platform time_t

jmclient/jmclient/wallet.py:2162: OverflowError
========================================================================= 3 failed, 284 passed in 2402.10s (0:40:02) =========================================================================
chris-belcher commented 3 years ago

Damn!

This is basically the year 2038 problem. That is only 17 years from now. It's plausible that JoinMarket would exist 10-20 years from now and/or that someone wants to lock up coins for a decade or two to use a fidelity bond. So it's not a good solution to just disable those dates.

Bitcoin itself has a similar problem in block headers but because it uses an unsigned integer the wraparound date is in the 22nd century. Is there a way we can have 32bit python use an unsigned integer?

AdamISZ commented 3 years ago

@chris-belcher would you be addressing this as part of your recent fidelity bond work?

chris-belcher commented 3 years ago

I don't know how. I haven't thought about it much yet.

chris-belcher commented 3 years ago

OK I did some searching and found a solution to the 2038 problem on python I think.

All we need to do is have a way to change unix timestamps into datetimes (year, month, day etc), and datetimes into timestamps.

https://stackoverflow.com/questions/10588027/converting-timestamps-larger-than-maxint-into-datetime-objects

>>> import datetime
>>> from calendar import timegm
>>> timegm(datetime.datetime(2040, 1, 1, 0, 0, 0, 0).timetuple()) ###converting (year,month) into unix time works
2208988800
>>> datetime.datetime.utcfromtimestamp(2208988800) ###converting that unix time to (year, month) fails on this 32 bit system
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: timestamp out of range for platform time_t
>>> datetime.datetime.utcfromtimestamp(0) + datetime.timedelta(seconds=2208988800) ###using this trick from the stackoverflow thread does work
datetime.datetime(2040, 1, 1, 1, 0)
>>> 

So it's actually a one line edit once you find the stackoverflow thread telling you what to do.

However this will require some testing because the stackoverflow thread says there might be timezone issues.

EDIT: the machine I'm testing this on is on the greenwich timezone, so its offset seems to be zero. Could someone else try it if they have a 32 bit platform on another timezone? All you do is run the following code and change the years/months/days/hours around to check that the resulting datetime always matches what you wrote.

>>> import datetime
>>> from calendar import timegm
>>> datetime.datetime.utcfromtimestamp(0) + datetime.timedelta(seconds=timegm(datetime.datetime(2070, 1, 1, 10, 0, 0, 0).timetuple()))
datetime.datetime(2070, 1, 1, 10, 0)

EDIT2: Looks like I can make timezone become an issue if I use datetime.fromtimestamp() instead of datetime.utcfromtimestamp. If so then the solution is to use the utcfromtimestamp function, and that's all we need. The fix really is a one line edit.