Page MenuHomePhabricator
Differential D2793

Merge #11041: Add LookupBlockIndex
ClosedPublic
Actions

Authored by jasonbcox on Apr 10 2019, 17:46.

Details

Reviewers
deadalnix
Fabien
Group Reviewers
Restricted Project
Commits
rSTAGING60d8a7a5a5ed: Merge #11041: Add LookupBlockIndex
rABC60d8a7a5a5ed: Merge #11041: Add LookupBlockIndex
Summary

92fabcd44 Add LookupBlockIndex function (João Barbosa)
43a32b739 Add missing cs_lock in CreateWalletFromFile (João Barbosa)
f814a3e8f Fix cs_main lock in LoadExternalBlockFile (João Barbosa)
c651df8b3 Lock cs_main while loading block index in AppInitMain (João Barbosa)
02de6a6bc Assert cs_main is held when accessing mapBlockIndex (João Barbosa)

Pull request description:

Replace all `mapBlockIndex` lookups with the new `LookupBlockIndex()`. In some cases it avoids a second lookup.

Tree-SHA512: ca31118f028a19721f2191d86f2dd398144d04df345694575a64aeb293be2f85785201480c3c578a0ec99690516205708558c0fd4168b09313378fd4e60a8412

Backport of Core PR 11041
https://github.com/bitcoin/bitcoin/pull/11041/files
Completes T560
Depends on D1971, D2801

Note to reviewers: LookupBlockIndex was moved from validation.h to chain.h to prevent unnecessary inclusions of validation.h in various files.

Test Plan
make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
pr11041
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5429
Build 8920: Bitcoin ABC Buildbot (legacy)
Build 8919: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Apr 10 2019, 17:46
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 10 2019, 17:46
teamcity added a comment.Apr 10 2019, 17:47

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

jasonbcox added inline comments.Apr 10 2019, 17:48
src/checkpoints.cpp
10

Re-adding here since it was removed in D712

src/validation.h
16

Adding this for BlockMap typedef, which was moved to chain.h in D712

teamcity added a comment.Apr 10 2019, 18:03

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

Harbormaster completed remote builds in B5429: Diff 8025.Apr 10 2019, 18:03
deadalnix requested changes to this revision.Apr 10 2019, 21:59
deadalnix added inline comments.
src/checkpoints.cpp
10

Including a giant header that define half the world for just one function is a good sign that function isn't where it should be.

src/validation.h
16

That was the second sign.

626

This very clearly belongs into chain.h

This revision now requires changes to proceed.Apr 10 2019, 21:59
jasonbcox updated this revision to Diff 8031.Apr 10 2019, 23:31

Move LookupBlockIndex definition to chain.h

teamcity added a comment.Apr 10 2019, 23:32

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

jasonbcox edited the summary of this revision. (Show Details)Apr 10 2019, 23:32
teamcity added a comment.Apr 10 2019, 23:47

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

Harbormaster completed remote builds in B5433: Diff 8031.Apr 10 2019, 23:47
Fabien requested changes to this revision.Apr 12 2019, 06:16
Fabien added inline comments.
src/rpc/rawtransaction.cpp
183 ↗(On Diff #8031)

This line is not in the original PR, it is removed in PR11838
https://github.com/bitcoin/bitcoin/pull/11838/files

This revision now requires changes to proceed.Apr 12 2019, 06:16
jasonbcox edited the summary of this revision. (Show Details)Apr 13 2019, 16:02
jasonbcox added a parent revision: D2801: Merge #11838: qa: Add getrawtransaction in_active_chain=False test.
jasonbcox updated this revision to Diff 8047.Apr 13 2019, 16:08

Fix dependency + rebase

teamcity added a comment.Apr 13 2019, 16:08

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

teamcity added a comment.Apr 13 2019, 16:22

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

Harbormaster completed remote builds in B5448: Diff 8047.Apr 13 2019, 16:22
deadalnix accepted this revision.Apr 14 2019, 17:11
deadalnix added inline comments.
src/init.cpp
2189 ↗(On Diff #8047)

ut everything in one string and let clang-format do its magic.

jasonbcox updated this revision to Diff 8052.Apr 14 2019, 17:20

Fix string linting + rebase

teamcity added a comment.Apr 14 2019, 17:21

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

teamcity added a comment.Apr 14 2019, 17:36

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

Harbormaster completed remote builds in B5452: Diff 8052.Apr 14 2019, 17:36
Fabien accepted this revision.Apr 15 2019, 07:48
This revision is now accepted and ready to land.Apr 15 2019, 07:48
Closed by commit rABC60d8a7a5a5ed: Merge #11041: Add LookupBlockIndex (authored by Wladimir J. van der Laan <laanwj@gmail.com>, committed by jasonbcox). · Explain WhyApr 15 2019, 22:38
This revision was automatically updated to reflect the committed changes.
jasonbcox added a commit: rSTAGING60d8a7a5a5ed: Merge #11041: Add LookupBlockIndex.Sep 18 2019, 22:57

Revision Contents
Changeset List

PathSize
src/
2 lines
7 lines
33 lines
86 lines
qt/
6 lines
8 lines
rpc/
37 lines
10 lines
31 lines
7 lines
85 lines
wallet/
7 lines
8 lines
36 lines

Diff 8025

View Options

src/checkpoints.h

Loading...
View Options

src/checkpoints.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/net_processing.cpp

Loading...
View Options

src/qt/transactionrecord.cpp

Loading...
View Options

src/rest.cpp

Loading...
View Options

src/rpc/blockchain.cpp

Loading...
View Options

src/rpc/mining.cpp

Loading...
View Options

src/rpc/rawtransaction.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...
View Options

src/wallet/rpcdump.cpp

Loading...
View Options

src/wallet/rpcwallet.cpp

Loading...
View Options

src/wallet/wallet.cpp

Loading...