New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change more 32 bit data fields to 64 bit #1206

Open
abitmore opened this Issue Jul 30, 2018 · 4 comments

Comments

5 participants
@abitmore
Member

abitmore commented Jul 30, 2018

Here are some, please add new findings.

  • account_statistics_object
    • total_ops
    • removed_ops
  • account_transaction_history_object
    • sequence

#1088 is related.

@jmjatlanta

This comment has been minimized.

jmjatlanta commented Oct 16, 2018

The items in the original post have been changed to 64 bit.

Un-assigning myself, and leaving this ticket open so we can keep track of other items that should be switched from 32 to 64 bit.

@jmjatlanta jmjatlanta removed their assignment Oct 16, 2018

@crypto-ape

This comment has been minimized.

crypto-ape commented Oct 17, 2018

Block timestamp is stored as an unsigned 32-bit value (block_header::timestamp).

This won't be an issue in near future but will cause failure by 2038 (if the source is 32-bit signed integer) or at the latest by 2106 (see year 2038 problem).

We don't need to worry about the operating system providing correct time (which it sure will) but about the BitShares' codebase. It seems the class time_point_sec representing the Unix Epoch time in fc library is fed by the class microseconds which stores its timestamp with an 64-bit integer. Thus if no bits are lost during the conversions and manipulations we are probably talking about the year 2106. Yet, simple comparisons in ifs can cause temporal overflows resulting in bugs occurring sooner.

Actually, all the Unix Epoch time manipulations by means of the class time_point_sec may (and will) fail, sooner or later.

Ape proposes the following:

  • Extend the internal field of the class time_point_sec to 64-bit unsigned integer and change the code appropriately.
  • Separately, inspect the time source and fix if necessary.
  • In order to save some space, leave the block header timestamp field as 32-bit until some (to be agreed) safe date (e.g. 2034).
@pmconrad

This comment has been minimized.

pmconrad commented Oct 17, 2018

Changing time representation now is a waste of time IMO. Can revisit the issue in 15 years.

@clockworkgr

This comment has been minimized.

Member

clockworkgr commented Oct 17, 2018

Changing time representation now is a waste of time IMO. Can revisit the issue in 15 years.

was gonna say...if this codebase is still around in 2038, we'll all probably be too rich to care :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment