Page MenuHomePhabricator
Differential D7988

rpc: Remove mempool global from miner
ClosedPublic
Actions

Authored by Fabien on Oct 19 2020, 18:32.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rABC97808ad06894: rpc: Remove mempool global from miner
Summary
The miner needs read-only access to the mempool. Instead of using the
mutable global ::mempool, keep a immutable reference to a mempool that
is passed to the miner. Apart from the obvious benefits of removing a
global and making things immutable, this might also simplify testing
with multiple mempools.

Backport of core PR17781.

Depends on D7984.

This differs quite a bit from the original PR because we already passed
a mempool. This is mostly rename and factorization to make it looks more
like core.

Test Plan
ninja all check-all
ninja bench-bitcoin

With clang:

ninja check

Diff Detail

Event Timeline

Fabien created this revision.Oct 19 2020, 18:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 19 2020, 18:32
Fabien requested review of this revision.Oct 19 2020, 18:32
teamcity edited the summary of this revision. (Show Details)Oct 19 2020, 18:32
Harbormaster completed remote builds in B13497: Diff 24792.Oct 19 2020, 18:42
Fabien edited the summary of this revision. (Show Details)Oct 19 2020, 20:44
Fabien updated this revision to Diff 24795.Oct 19 2020, 20:44
Fabien edited the summary of this revision. (Show Details)

Rebase

Fabien removed a parent revision: D7984: test: Add generatetodescriptor RPC.Oct 19 2020, 20:45
Harbormaster completed remote builds in B13498: Diff 24795.Oct 19 2020, 21:02
Fabien added a child revision: D7990: refactor: Pass NodeContext to RPC and REST methods through util::Ref.Oct 19 2020, 21:24
jasonbcox accepted this revision.Oct 19 2020, 23:36
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/miner.h
169 ↗(On Diff #24795)

Nit: Make it match Core: _mempool -> mempool

This revision is now accepted and ready to land.Oct 19 2020, 23:36
Fabien added inline comments.Oct 20 2020, 06:15
src/miner.h
169 ↗(On Diff #24795)

Good catch, I did it for the cpp file but missed the header

Fabien updated this revision to Diff 24811.Oct 20 2020, 06:19
Fabien added a parent revision: D7984: test: Add generatetodescriptor RPC.

_mempool => mempool

Harbormaster completed remote builds in B13511: Diff 24811.Oct 20 2020, 06:29
teamcity added a comment.Oct 20 2020, 06:30
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Tail of the build log:

rpc_getchaintips.py                              | ✓ Passed  | 2 s
rpc_help.py                                      | ✓ Passed  | 1 s
rpc_invalidateblock.py                           | ✓ Passed  | 1 s
rpc_misc.py                                      | ✓ Passed  | 1 s
rpc_named_arguments.py                           | ✓ Passed  | 1 s
rpc_net.py                                       | ✓ Passed  | 1 s
rpc_preciousblock.py                             | ✓ Passed  | 1 s
rpc_psbt.py                                      | ✓ Passed  | 28 s
rpc_rawtransaction.py                            | ✓ Passed  | 45 s
rpc_scantxoutset.py                              | ✓ Passed  | 4 s
rpc_setban.py                                    | ✓ Passed  | 2 s
rpc_signmessage.py                               | ✓ Passed  | 1 s
rpc_signrawtransaction.py                        | ✓ Passed  | 1 s
rpc_txoutproof.py                                | ✓ Passed  | 2 s
rpc_uptime.py                                    | ✓ Passed  | 1 s
rpc_users.py                                     | ✓ Passed  | 4 s
rpc_whitelist.py                                 | ✓ Passed  | 1 s
tool_wallet.py                                   | ✓ Passed  | 3 s
wallet_abandonconflict.py                        | ✓ Passed  | 10 s
wallet_address_types.py                          | ✓ Passed  | 15 s
wallet_avoidreuse.py                             | ✓ Passed  | 6 s
wallet_backup.py                                 | ✓ Passed  | 46 s
wallet_balance.py                                | ✓ Passed  | 9 s
wallet_basic.py                                  | ✓ Passed  | 26 s
wallet_coinbase_category.py                      | ✓ Passed  | 1 s
wallet_create_tx.py                              | ✓ Passed  | 5 s
wallet_createwallet.py                           | ✓ Passed  | 2 s
wallet_createwallet.py --usecli                  | ✓ Passed  | 2 s
wallet_disable.py                                | ✓ Passed  | 1 s
wallet_dump.py                                   | ✓ Passed  | 2 s
wallet_encryption.py                             | ✓ Passed  | 6 s
wallet_groups.py                                 | ✓ Passed  | 44 s
wallet_hd.py                                     | ✓ Passed  | 6 s
wallet_import_rescan.py                          | ✓ Passed  | 5 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 2 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 2 s
wallet_labels.py                                 | ✓ Passed  | 1 s
wallet_listreceivedby.py                         | ✓ Passed  | 8 s
wallet_listsinceblock.py                         | ✓ Passed  | 2 s
wallet_listtransactions.py                       | ✓ Passed  | 14 s
wallet_multiwallet.py                            | ✓ Passed  | 14 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 15 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 11 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 4 s

ALL                                              | ✓ Passed  | 766 s (accumulated) 
Runtime: 154 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1
This revision was landed with ongoing or failed builds.Oct 20 2020, 06:33
Closed by commit rABC97808ad06894: rpc: Remove mempool global from miner (authored by MarcoFalke <falke.marco@gmail.com>, committed by Fabien). · Explain Why
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC97808ad06894: rpc: Remove mempool global from miner.

Revision Contents
Changeset List

PathSize
src/
bench/
4 lines
5 lines
6 lines
6 lines
12 lines
33 lines
rpc/
14 lines
test/
36 lines
84 lines
util/
9 lines
15 lines
85 lines

Diff 24811

View Options

src/bench/bench.h

Loading...
View Options

src/bench/bench.cpp

Loading...
View Options

src/bench/block_assemble.cpp

Loading...
View Options

src/bench/wallet_balance.cpp

Loading...
View Options

src/miner.h

Loading...
View Options

src/miner.cpp

Loading...
View Options

src/rpc/mining.cpp

Loading...
View Options

src/test/blockfilter_index_tests.cpp

Loading...
View Options

src/test/miner_tests.cpp

Loading...
View Options

src/test/util/mining.h

Loading...
View Options

src/test/util/mining.cpp

Loading...
View Options

src/test/validation_block_tests.cpp

Loading...