Page MenuHomePhabricator

Add nTimeReceived to CBlockIndex for tracking recevied time of blocks
ClosedPublic

Authored by jasonbcox on Apr 14 2018, 17:15.

Details

Summary

D1196 for prototype implementation. Partial work for T208.
nTimeReceived will be used to break ties for blocks as the same height.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Apr 14 2018, 17:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 14 2018, 17:15
deadalnix added inline comments.Apr 15 2018, 20:52
src/chain.h
323 ↗(On Diff #3505)

Maybe it's better to put the 'right' value for the field when deserializing ?

deadalnix requested changes to this revision.Apr 15 2018, 20:53

Also, we should probably serialize/deserialize this from disk as well, but this can wait.

This revision now requires changes to proceed.Apr 15 2018, 20:53
jimpo added a subscriber: jimpo.Apr 16 2018, 18:37
jimpo added inline comments.
src/chain.h
237 ↗(On Diff #3505)

Move this below nSequenceId for better alignment. Putting it here adds 4 bytes of padding above it in the struct. You can see by inspecting (char*)(&block_index.nTimeReceived) - (char*)(&block_index) with nTimeReceived in this location or below. On my machine, the offset is the same.

Optimizing alignment on this struct is important because there are many of them stored in memory all the time.

Also, we should probably serialize/deserialize this from disk as well, but this can wait.

T196 appears to be the beginning of this discussion. Please comment if you have any additional thoughts.

jasonbcox updated this revision to Diff 3526.Apr 17 2018, 02:56
jasonbcox marked 2 inline comments as done.

Fixed according to feedback.

jasonbcox added inline comments.Apr 17 2018, 02:58
src/chain.h
237 ↗(On Diff #3505)

Good callout. Fixed.

323 ↗(On Diff #3505)

By deserializing, I assume you mean in the constructor that takes CBlockHeader as input, since I couldn't find any other deserialization functions for CBlockIndex. Done.

This also means that nTimeReceived being 0 would be easier to debug since it's expect to be set when a new CBlockIndex is initialized from a non-CBlockHeader source. I think this is a good thing.

jasonbcox marked 2 inline comments as done.Apr 17 2018, 02:58
deadalnix accepted this revision.Apr 18 2018, 23:12

For de-serialization, I see that it comes from the block header (only the block header is serialized/deserialized), so there is no way to do what I wanted, at least not easily. So let's not block that diff for it.

src/chain.h
323 ↗(On Diff #3505)

Ok.

This revision is now accepted and ready to land.Apr 18 2018, 23:12
This revision was automatically updated to reflect the committed changes.