Page MenuHomePhabricator

[3 of 5] Move CDiskBlockPos from chain to flatfile.
ClosedPublic

Authored by markblundeberg on Jul 7 2019, 22:31.

Details

Summary

partial PR15118 backport
https://github.com/bitcoin/bitcoin/pull/15118/commits/d6d8a78f26f52fdfe43293686135e2fc6919926c

Backport note: something similar in spirit was done in D1655, but
conflicts with how Core ended up doing it. This overrides that change
and syncs to how Core does it currently. The class itself is unchanged
except for the ToString method being moved to cpp file.

Depends on D3586

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

markblundeberg created this revision.Jul 7 2019, 22:31
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 7 2019, 22:31
markblundeberg added inline comments.Jul 7 2019, 22:34
src/test/script_p2sh_tests.cpp
14 ↗(On Diff #10065)

needed due to tinyformat.h no longer being included indirectly

Note: alternatively we could keep the split and rename diskblockpos.h to flatfilepos.h and put the ToString() implementation in a .cpp file also, or ...?

I'm not totally sure if it's worth splitting up the two, the main thing seems to have been getting it out of chain.h.

rebase for minor conflict

nakihito added a comment.Jul 9 2019, 20:43

Note: alternatively we could keep the split and rename diskblockpos.h to flatfilepos.h and put the ToString() implementation in a .cpp file also, or ...?
I'm not totally sure if it's worth splitting up the two, the main thing seems to have been getting it out of chain.h.

For now I'm in favor of merging the two since it would make backports easier. After we've caught up, then it may be worthwhile to reconsider.

rebased parent & new abc ver

Note: alternatively we could keep the split and rename diskblockpos.h to flatfilepos.h and put the ToString() implementation in a .cpp file also, or ...?
I'm not totally sure if it's worth splitting up the two, the main thing seems to have been getting it out of chain.h.

For now I'm in favor of merging the two since it would make backports easier. After we've caught up, then it may be worthwhile to reconsider.

Right, splitting it up means we now own the code to an extent. There's no reason for us to take ownership of this at this point in time.

jasonbcox accepted this revision.Jul 16 2019, 21:32
This revision is now accepted and ready to land.Jul 16 2019, 21:32
nakihito accepted this revision.Jul 16 2019, 23:09