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

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

Details

Reviewers

deadalnix
jasonbcox
Fabien
nakihito

Group Reviewers

Restricted Project

Maniphest Tasks

T631: Backport PR 15118 "Refactor block file logic"

Commits

rSTAGINGc2a03e1e9547: [3 of 5] Move CDiskBlockPos from chain to flatfile.
rABCc2a03e1e9547: [3 of 5] Move CDiskBlockPos from chain to flatfile.

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

markblundeberg created this revision.Jul 7 2019, 22:31

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 7 2019, 22:31

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg added a task: T631: Backport PR 15118 "Refactor block file logic".Jul 7 2019, 22:31

markblundeberg added inline comments.Jul 7 2019, 22:34

src/test/script_p2sh_tests.cpp
14	needed due to tinyformat.h no longer being included indirectly

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6720: Diff 10065.Jul 7 2019, 22:39

markblundeberg added a child revision: D3588: [4 of 5] scripted-diff: Rename CBlockDiskPos to FlatFilePos..Jul 7 2019, 22:41

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

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B6772: Diff 10153.Jul 9 2019, 19:14

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

In D3587#84811, @markblundeberg wrote:

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

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B6849: Diff 10277!Jul 13 2019, 02:07

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B6849: Diff 10277.Jul 13 2019, 03:05

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

In D3587#85317, @nakihito wrote:

In D3587#84811, @markblundeberg wrote:

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.