Skip to content
This repository has been archived by the owner on Jun 27, 2022. It is now read-only.

Feature/timestamp #63

Merged
merged 6 commits into from
Feb 8, 2018
Merged

Feature/timestamp #63

merged 6 commits into from
Feb 8, 2018

Conversation

amougel
Copy link
Contributor

@amougel amougel commented Feb 5, 2018

Adding timestamp management to prepare for future altcoins integration

@amougel amougel requested review from gre and meriadec February 5, 2018 14:43
this.toHexDigit((number >> 8) & 0xff) +
this.toHexDigit(number & 0xff)
);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can these 2 be implemented using Buffer instead?

something like buffer=Buffer.alloc(4); buffer.writeUInt32LE(number, 0); return buffer.toString("hex") (not tested) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was just transcribing the code (not trying to improve it right away). I'll fix it.

.reverse()
.join(""),
"hex"
))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better if we can directly create the Buffer with the number. without using a temporary string conversion

@amougel amougel force-pushed the feature/timestamp branch 12 times, most recently from 1662b71 to 1384381 Compare February 8, 2018 13:50
@gre gre merged commit 38610b8 into LedgerHQ:master Feb 8, 2018
@amougel amougel deleted the feature/timestamp branch February 9, 2018 16:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants