Page MenuHomePhabricator

Merge #11293: Deduplicate CMerkleBlock construction code, add test coverage
Changes PlannedPublic

Authored by nakihito on Jul 17 2019, 20:51.

Details

Reviewers
deadalnix
Fabien
jasonbcox
markblundeberg
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

46ce223d1 Add tests for CMerkleBlock usage with txids specified (James O'Beirne)
5ab586f90 Consolidate CMerkleBlock constructor into a single method (James O'Beirne)

Pull request description:

What started as a simple task to add test coverage ended up giving way to a light refactoring. This consolidates the mostly-identical `CMerkleBlock` constructors into one (using C++11 constructor delegation) and adds coverage for the by-txids construction case.

### Before

![selection_006](https://user-images.githubusercontent.com/73197/30242104-0f381fe4-9545-11e7-9617-83b87fce0456.png)

### After

![selection_008](https://user-images.githubusercontent.com/73197/30242107-1425dfaa-9545-11e7-9e6b-2c3432517dd1.png)

Tree-SHA512: eed84ed3e8bfc43473077b575c8252759a857e37275e4b36ca7cc2c17a65895e5f494bfd9d4aeab09fc6e98fc6a9c641ac7ecc0ddbeefe01a9e4308e7909e529

Backport of Core PR11293
https://github.com/bitcoin/bitcoin/pull/11293/

Adjusted to take into account D3371

Test Plan
make check
ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR11293
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6886
Build 11819: Bitcoin ABC Teamcity Staging
Build 11818: arc lint + arc unit

Event Timeline

nakihito created this revision.Jul 17 2019, 20:51
Owners added a reviewer: Restricted Owners Package.Jul 17 2019, 20:51
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 17 2019, 20:51
deadalnix requested changes to this revision.Jul 18 2019, 12:06

cmake build

This revision now requires changes to proceed.Jul 18 2019, 12:06
nakihito updated this revision to Diff 10349.Jul 18 2019, 18:05

Added CMake.

nakihito edited the test plan for this revision. (Show Details)Jul 19 2019, 01:06
Fabien accepted this revision.Jul 23 2019, 10:20
deadalnix added inline comments.Wed, Aug 28, 17:41
src/merkleblock.cpp
24

This clearly isn't CTOR compliant.

jasonbcox requested changes to this revision.Mon, Sep 9, 22:06

Clearing this off my queue. See Amaury's comment about CTOR.

This revision now requires changes to proceed.Mon, Sep 9, 22:06
nakihito planned changes to this revision.Wed, Sep 11, 20:12