[net_processing] Pass chainparams to PeerLogicValidation constructor
ClosedPublic
Actions

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

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rABC63c9d13451b6: [net_processing] Pass chainparams to PeerLogicValidation constructor

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

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Jan 18 2021, 13:26

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 18 2021, 13:26

Fabien requested review of this revision.Jan 18 2021, 13:26

teamcity edited the summary of this revision. (Show Details)Jan 18 2021, 13:26

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 edited the summary of this revision. (Show Details)Jan 18 2021, 13:51

@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.

Harbormaster completed remote builds in B14820: Diff 27038.Jan 18 2021, 14:01

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

Fabien edited the summary of this revision. (Show Details)Jan 19 2021, 08:54

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

Harbormaster completed remote builds in B14833: Diff 27060.Jan 19 2021, 09:04

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

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

Back to your queue due to test failures.

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

Rebase

Harbormaster completed remote builds in B14917: Diff 27206.Jan 21 2021, 18:41

deadalnix accepted this revision.Jan 21 2021, 20:51

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

Closed by commit rABC63c9d13451b6: [net_processing] Pass chainparams to PeerLogicValidation constructor (authored by John Newbery <john@johnnewbery.com>, committed by Fabien). · Explain WhyJan 22 2021, 11:11

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC63c9d13451b6: [net_processing] Pass chainparams to PeerLogicValidation constructor.