Page MenuHomePhabricator

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

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

Details

Reviewers
deadalnix
jasonbcox
Fabien
nakihito
Group Reviewers
Restricted Project
Maniphest Tasks
T631: Backport PR 15118 "Refactor block file logic"
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

Event Timeline

markblundeberg created this revision.Sun, Jul 7, 22:31
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Jul 7, 22:31
markblundeberg added inline comments.Sun, Jul 7, 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.Tue, Jul 9, 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.Tue, Jul 16, 21:32
This revision is now accepted and ready to land.Tue, Jul 16, 21:32
nakihito accepted this revision.Tue, Jul 16, 23:09