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
Branch
PR15118c
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6720
Build 11487: Bitcoin ABC Buildbot (legacy)
Build 11486: arc lint + arc unit

Event Timeline

src/test/script_p2sh_tests.cpp
14

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.

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.

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.

This revision is now accepted and ready to land.Jul 16 2019, 21:32