Page MenuHomePhabricator

Move primitives/transaction.cpp to common for CMake
AbandonedPublic

Authored by nakihito on Nov 19 2019, 19:52.

Details

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

There is a circular dependency that is currently hidden in CMake builds.
chainparams.cpp from common requires primities/transactions.cpp
from bitcoinconsensus which also depends on common. This is
hidden in every other build by linking common before linking
bitcoinconsensus.

By moving primitives/transaction.cpp from bitcoinconsensus to
common, we can keep it and primitives/block.cpp together in one
library. This will remove the circular dependency mentioned above.

Discussed previously with Fabien.

Test Plan
ninja
ninja check
ninja bitcoin-bench

In the cmake build's src directory after successful building:

nm -D --defined-only libbitcoinconsensus.so

Confirm that the following lines appear:

0000000000009100 T bitcoinconsensus_verify_script
00000000000090b0 T bitcoinconsensus_verify_script_with_amount
0000000000008170 T bitcoinconsensus_version

Confirm gitian builds are successful

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ChangeCMakebitcoinconsensus
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8165
Build 14362: Default Diff Build & Tests
Build 14361: arc lint + arc unit

Event Timeline

nakihito created this revision.Nov 19 2019, 19:52
Owners added a reviewer: Restricted Owners Package.Nov 19 2019, 19:52
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 19 2019, 19:52
nakihito planned changes to this revision.Nov 19 2019, 19:52
nakihito edited the summary of this revision. (Show Details)Nov 19 2019, 23:50
nakihito edited the test plan for this revision. (Show Details)
nakihito edited the test plan for this revision. (Show Details)Nov 20 2019, 18:16
nakihito requested review of this revision.Nov 20 2019, 18:55
nakihito abandoned this revision.Nov 22 2019, 19:40

Discussed offline.