Page MenuHomePhabricator

[net_processing] Pass chainparams to PeerLogicValidation constructor
ClosedPublic

Authored by Fabien on Jan 18 2021, 13:26.

Details

Summary
Keep a references to chainparams, rather than calling the global
Params() function every time it's needed. This is fine, since
globalChainParams does not get updated once it's been set, and it's
available at the point of constructing the PeerLogicValidation object.

Partial backport of core PR19791 (2/9):
https://github.com/bitcoin/bitcoin/pull/19791/commits/2297b26b3ce95e935c0ebb8c38dabf19965054a5

Test Plan
ninja all check-all

Event Timeline

Fabien requested review of this revision.Jan 18 2021, 13:26
deadalnix requested changes to this revision.Jan 18 2021, 13:27
deadalnix added a subscriber: deadalnix.

You say that it differs from the original by passing a config, but provide no reason as to why. It doesn't looks like to me that this difference is justified.

This revision now requires changes to proceed.Jan 18 2021, 13:27
Fabien requested review of this revision.Jan 18 2021, 13:53

@deadalnix I added a line with the rationale in the summary:
The config is needed down the road, and provides a facility to retrieve the chainparams, so storing it makes it possible to have a simpler API..
Let me know if you think it's not worth it.

deadalnix requested changes to this revision.Jan 18 2021, 21:58

The config is needed down the road is just saying nothing at all.

This revision now requires changes to proceed.Jan 18 2021, 21:58

Stick with original PR content.
API optimization can still happen later if needed.

Tail of the build log:

[371/430] Running utility command for check-bitcoin-wallet_tests
[372/430] bitcoin: testing sync_tests
[373/430] Running utility command for check-bitcoin-finalization_tests
[374/430] bitcoin: testing sighashtype_tests
[375/430] bitcoin: testing bip32_tests
[376/430] Running utility command for check-bitcoin-script_commitment_tests
[377/430] Running utility command for check-bitcoin-merkleblock_tests
[378/430] Running utility command for check-bitcoin-sync_tests
[379/430] bitcoin: testing torcontrol_tests
[380/430] Running utility command for check-bitcoin-sighashtype_tests
[381/430] Running utility command for check-bitcoin-bip32_tests
[382/430] Running utility command for check-bitcoin-torcontrol_tests
[383/430] bitcoin: testing settings_tests
[384/430] Running utility command for check-bitcoin-settings_tests
[385/430] bitcoin: testing uint256_tests
[386/430] bitcoin: testing streams_tests
[387/430] Running utility command for check-bitcoin-uint256_tests
[388/430] Running utility command for check-bitcoin-streams_tests
[389/430] bitcoin: testing walletdb_tests
[390/430] Running utility command for check-bitcoin-walletdb_tests
[391/430] bitcoin: testing undo_tests
[392/430] bitcoin: testing validation_flush_tests
[393/430] bitcoin: testing util_threadnames_tests
[394/430] Running utility command for check-bitcoin-validation_flush_tests
[395/430] Running utility command for check-bitcoin-undo_tests
[396/430] Running utility command for check-bitcoin-util_threadnames_tests
[397/430] bitcoin: testing compilerbug_tests
[398/430] Running utility command for check-bitcoin-compilerbug_tests
[399/430] bitcoin: testing serialize_tests
[400/430] Running utility command for check-bitcoin-serialize_tests
[401/430] bitcoin: testing ismine_tests
[402/430] bitcoin: testing getarg_tests
[403/430] bitcoin: testing radix_tests
[404/430] Running utility command for check-bitcoin-ismine_tests
[405/430] Running utility command for check-bitcoin-getarg_tests
[406/430] Running utility command for check-bitcoin-radix_tests
[407/430] bitcoin: testing ref_tests
[408/430] Running utility command for check-bitcoin-ref_tests
[409/430] bitcoin: testing schnorr_tests
[410/430] bitcoin: testing crypto_tests
[411/430] Running utility command for check-bitcoin-schnorr_tests
[412/430] bitcoin: testing blockcheck_tests
[413/430] Running utility command for check-bitcoin-crypto_tests
[414/430] Running utility command for check-bitcoin-blockcheck_tests
[415/430] bitcoin: testing monolith_opcodes_tests
[416/430] Running utility command for check-bitcoin-monolith_opcodes_tests
[417/430] bitcoin: testing validation_tests
[418/430] Running utility command for check-bitcoin-validation_tests
[419/430] bitcoin: testing cuckoocache_tests
[420/430] Running utility command for check-bitcoin-cuckoocache_tests
[421/430] bitcoin: testing skiplist_tests
[422/430] Running utility command for check-bitcoin-skiplist_tests
[423/430] bitcoin: testing op_reversebytes_tests
[424/430] Running utility command for check-bitcoin-op_reversebytes_tests
[425/430] bitcoin: testing transaction_tests
[426/430] Running utility command for check-bitcoin-transaction_tests
[427/430] bitcoin: testing coins_tests
[428/430] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
deadalnix requested changes to this revision.Jan 21 2021, 18:12

Back to your queue due to test failures.

This revision now requires changes to proceed.Jan 21 2021, 18:12
This revision is now accepted and ready to land.Jan 21 2021, 20:51
Fabien retitled this revision from [net_processing] Pass config to PeerLogicValidation constructor to [net_processing] Pass chainparams to PeerLogicValidation constructor.Jan 22 2021, 10:43