Age | Commit message (Collapse) | Author |
|
in only one translation unit
0959d37e3e Don't use global (external) symbols for symbols that are used in only one translation unit (practicalswift)
Pull request description:
Don't use global (external) symbols for symbols that are used in only one translation unit.
Before:
```
$ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
N_REFERENCES=$(wc -l <<< "${REFERENCES}")
if [[ ${N_REFERENCES} > 1 ]]; then
continue
fi
echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
done
Global symbol g_chainstate is used in only one translation unit: src/validation.cpp
Global symbol g_ui_signals is used in only one translation unit: src/ui_interface.cpp
Global symbol instance_of_cmaincleanup is used in only one translation unit: src/validation.cpp
Global symbol instance_of_cnetcleanup is used in only one translation unit: src/net.cpp
Global symbol instance_of_cnetprocessingcleanup is used in only one translation unit: src/net_processing.cpp
Global symbol pindexBestForkBase is used in only one translation unit: src/validation.cpp
Global symbol pindexBestForkTip is used in only one translation unit: src/validation.cpp
$
```
After:
```
$ for SYMBOL in $(nm src/bitcoind | grep -E ' [BD] ' | c++filt | cut -f3- -d' ' | grep -v @ | grep -v : | sort | grep '[a-z]' | sort -u | grep -vE '(^_|typeinfo|vtable)'); do
REFERENCES=$(git grep -lE "([^a-zA-Z]|^)${SYMBOL}([^a-zA-Z]|\$)" -- "*.cpp" "*.h")
N_REFERENCES=$(wc -l <<< "${REFERENCES}")
if [[ ${N_REFERENCES} > 1 ]]; then
continue
fi
echo "Global symbol ${SYMBOL} is used in only one translation unit: ${REFERENCES}"
done
$
```
♻️ Think about future generations: save the global namespace from unnecessary pollution! ♻️
ACKs for commit 0959d3:
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/16092/commits/0959d37e3e0f80010a78d175e3846dabf5d35919
MarcoFalke:
ACK 0959d37e3e0f80010a78d175e3846dabf5d35919
hebasto:
ACK 0959d37e3e0f80010a78d175e3846dabf5d35919
promag:
ACK 0959d37.
Tree-SHA512: 722f66bb50450f19b57e8a8fbe949f30cd651eb8564e5787cbb772a539bf3a288c048dc49e655fd73ece6a46f6dafade515ec4004729bf2b3ab83117b7c5d153
|
|
At this point there is no reasonable excuse to disable opt-in RBF,
and, unlike when this option was added, there are now significant
issues created when disabling it (in the form of compact block
reconstruction failures). Further, it breaks a lot of modern wallet
behavior.
|
|
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
|
|
translation unit
|
|
403e677c9 refactoring: IsInitialBlockDownload -> CChainState (James O'Beirne)
3ccbc376d refactoring: FlushStateToDisk -> CChainState (James O'Beirne)
4d6688603 refactoring: introduce ChainstateActive() (James O'Beirne)
d7c97edee move-only: make the CChainState interface public (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
This changeset starts moving functionality intimately related to CChainState into methods. Parameterizing these functions by a particular CChainState is necessary for the use of multiple chainstates simultaneously (e.g. for asynchronous background validation).
In this change, we
- make the CChainState interface public - since other units will start to invoke its methods directly,
- introduce `::ChainstateActive()`, the CChainState equivalent for `::ChainActive()`,
- and move `IsInitialBlockDownload()` and `FlushStateToDisk()` into methods on CChainState.
Independent of assumeutxo, these changes better encapsulate chainstate behavior and allow easier use from a testing context.
There are more methods that we'll move in the future, but they require other substantial changes (i.e. moving ownership of the `CCoinsView*` hierarchy into CChainState) so we'll save them for future PRs.
---
The first move-only commit is most easily reviewed with `git diff ... --color-moved=dimmed_zebra`.
ACKs for commit 403e67:
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/15976/commits/403e677c9ebbf9744733010e6b0c2d1b182ee850 no need to address my nits herein
Sjors:
utACK 403e677
ryanofsky:
utACK 403e677c9ebbf9744733010e6b0c2d1b182ee850. Only change since previous review is removing global state comment as suggested.
MarcoFalke:
utACK 403e677c9e, though the diff still seems a bit bloated with some unnecessary changes in the second commit.
promag:
utACK 403e677 and rebased with current [master](c7cfd20a7).
Tree-SHA512: 6fcf260bb2dc201361170c0b4547405366f5f331fcc3a2bac29b24442814b7b244ca1b58aac5af716885f9a130c343b544590dff780da0bf835c7c5b3ccb2257
|
|
|
|
RewindBlockIndex
1609809fb2 validation: Hold cs_main when reading chainActive in RewindBlockIndex (practicalswift)
Pull request description:
Fixes #15980.
Hold `cs_main` when reading `chainActive` (via `::ChainActive()`) in `RewindBlockIndex`.
ACKs for commit 160980:
MarcoFalke:
utACK 1609809fb2a4c2ec15b7c26dc328e2e666bd5d57
Tree-SHA512: 54f180ab391f92f04950735c2bb337f0b7495826d2096f7a0f9a2da50bc29d08747f404a0495e33ca4edd4c842efbab4c4730d5e1a8b9da3e1249cf884268f4b
|
|
af5fa82b6 Allow quicker shutdowns during LoadBlockIndex() (Jonas Schnelli)
Pull request description:
ACKs for commit af5fa8:
promag:
utACK af5fa82b676a36e60eda080ca0a946bdfffefd49.
practicalswift:
utACK af5fa82b676a36e60eda080ca0a946bdfffefd49
Tree-SHA512: 1c64dcc5d8a9d3411553257cd5a598dcd29be981660e5bca9283c1d957dc56798abcf41d9969cd573088137597a23e48e62a8c476c463d3f176b86a10048f47b
|
|
fa86c8aec6 init: Remove dead code in LoadChainTip (MarcoFalke)
Pull request description:
`LoadChainTip` sets `::ChainActive()` based on `pcoinsTip`'s best block. `LoadChainTip` is never called when that block is null, so we can remove all code from within that method that is only executed when that block is null.
Fixes #15967 Inconsistent locking behavior in LoadChainTip
ACKs for commit fa86c8:
promag:
utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d.
practicalswift:
utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
laanwj:
utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
ryanofsky:
utACK fa86c8aec611a9b9d2f53960c92419cf2a8bb92d. LoadChainTip isn't called currently when pcoinsTip best block is null due to this line:
jamesob:
utACK https://github.com/bitcoin/bitcoin/pull/15999/commits/fa86c8aec611a9b9d2f53960c92419cf2a8bb92d
Tree-SHA512: 8961c0e579800a52038ac5655478468852faac055299b64d6cfdf0c213d3bf09669c4889467d09d93457f6c8b073967bb0475a137f77ddd3a3a3c03ad90001c4
|
|
We introduce CChainState.m_cached_finished_ibd because the static state it
replaces would've been shared across all CChainState instances.
|
|
Also renames global methods for clarity:
- ::FlushStateToDisk() -> CChainState::ForceFlushStateToDisk()
- This performs an unconditional flush.
- ::PruneAndFlush() -> CChainState::PruneAndFlush()
|
|
To be used once we move global functions (e.g. FlushStateToDisk()) into
CChainState methods.
Thanks to Marco Falke for suggestions
|
|
along with DisconnectResult, and CBlockIndexWorkComparator.
The CChainState interface needs to be known to the rest of the system because
many global functions will move to CChainState methods. This is to allow
other parts of the system to be parameterized per chainstate instance
instead of assuming a single global.
|
|
|
|
|
|
|
|
requirement in LimitValidationInterfaceQueue
62d50ef308 Add LOCKS_EXCLUDED(cs_main) to LimitValidationInterfaceQueue(...) which does AssertLockNotHeld(cs_main) (practicalswift)
Pull request description:
This PR adds compile-time checking for negative locking requirements that follow from the run-time locking requirement `AssertLockNotHeld(cs_main)` in `LimitValidationInterfaceQueue(...)`.
Changes:
* Add `LOCKS_EXCLUDED(cs_main)` to `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `CChainState::ActivateBestChain(…)`, `CChainState:: InvalidateBlock(…)` and `CChainState::RewindBlockIndex(…)` which all call `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `InvalidateBlock(…)` which calls `CChainState::InvalidateBlock(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
* Add `LOCKS_EXCLUDED(cs_main)` to `RewindBlockIndex(…)` which calls `CChainState::RewindBlockIndex(...)` which in turn calls `LimitValidationInterfaceQueue(...)` which does `AssertLockNotHeld(cs_main)`
ACKs for commit 62d50e:
MarcoFalke:
utACK 62d50ef308
Tree-SHA512: 73d092ccd08c851ae3c5d60370c369fc030c5793f5507e2faccb6f91c851ddc0ce059fbea3899f2856330d7a8c78f2ac6a2988e8268b03154f946be9e60e3be1
|
|
AssertLockNotHeld(cs_main)
Add LOCKS_EXCLUDED(cs_main) to functions calling LimitValidationInterfaceQueue(...) which does AssertLockNotHeld(cs_main)
Add LOCKS_EXCLUDED(cs_main) to functions calling CChainState::InvalidateBlock(...) which calls LimitValidationInterfaceQueue(...) which in turn does AssertLockNotHeld(cs_main)
Add LOCKS_EXCLUDED(cs_main) to functions calling CChainState::RewindBlockIndex(...) which calls LimitValidationInterfaceQueue(...) which in turn does AssertLockNotHeld(cs_main)
|
|
486c1eea86 refactoring: remove unused chainActive (James O'Beirne)
631940aab2 scripted-diff: replace chainActive -> ::ChainActive() (James O'Beirne)
a3a609079c refactoring: introduce unused ChainActive() (James O'Beirne)
1b6e6fcfd2 rename: CChainState.chainActive -> m_chain (James O'Beirne)
Pull request description:
This is part of the assumeutxo project:
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
---
This change refactors the `chainActive` reference into a `::ChainActive()` call. It also distinguishes `CChainState`'s `CChain` data member as `m_chain` instead of the current `chainActive`, which makes it easily confused with the global data.
The active chain must be obtained via function because its reference will be swapped at some point during runtime after loading a UTXO snapshot.
This change, though lengthy, should be pretty easy to review since most of it is contained within a scripted-diff. Once merged, the parent PR should be easier to review.
ACKs for commit 486c1e:
Sjors:
utACK 486c1ee
promag:
utACK 486c1ee.
practicalswift:
utACK 486c1eea863a41e597ae4fddc392f446f2518b4b
Tree-SHA512: 06ed8f9e77f2d25fc9bea0ba86436d80dbbce90a1e8be23e37ec4eeb26060483e60b4a5c4fba679cb1867f61e3921c24abeb9cabdfb4d0a9b1c4ddd77b17456a
|
|
0ff1c2a838da9e8dc7f77609adc89124bbea3e2b Separate reason for premature spends (coinbase/locktime) (Suhas Daftuar)
54470e767bab37f9b7089782b1be73d5883bb244 Assert validation reasons are contextually correct (Suhas Daftuar)
2120c31521aa51aa1984ee33250b8320506d3a0f [refactor] Update some comments in validation.cpp as we arent doing DoS there (Matt Corallo)
12dbdd7a41bac73e51ed8f7b290b7671196bf9ea [refactor] Drop unused state.DoS(), state.GetDoS(), state.CorruptionPossible() (Matt Corallo)
aa502b88d10c2c3ac56d9163555849b96dc4df1e scripted-diff: Remove DoS calls to CValidationState (Matt Corallo)
7721ad64f40a0c67edefaaf7353264d78df8803e [refactor] Prep for scripted-diff by removing some \ns which annoy sed. (Matt Corallo)
5e78c5734bb0c9aae7b0a7019a745b2d7059b3d9 Allow use of state.Invalid() for all reasons (Matt Corallo)
6b34bc6b6f54f85537494cbea3846d5d195a06d9 Fix handling of invalid headers (Suhas Daftuar)
ef54b486d5333dfc85c56e6b933c81735196a25d [refactor] Use Reasons directly instead of DoS codes (Matt Corallo)
9ab2a0412e96e87956fe61257387683635213035 CorruptionPossible -> BLOCK_MUTATED (Matt Corallo)
6e55b292b0ea944897b6dc2f766446fd209af484 CorruptionPossible -> TX_WITNESS_MUTATED (Matt Corallo)
7df16e70e67c753c871797ce947ea09d7cb0e519 LookupBlockIndex -> CACHED_INVALID (Matt Corallo)
c8b0d22698385f91215ce8145631e3d5826dc977 [refactor] Drop redundant nDoS, corruptionPossible, SetCorruptionPossible (Matt Corallo)
34477ccd39a8d4bfa8ad612f22d5a46291922185 [refactor] Add useful-for-dos "reason" field to CValidationState (Matt Corallo)
6a7f8777a0b193fae4f976196f3464ffac01bf1b Ban all peers for all block script failures (Suhas Daftuar)
7b999103e21509e1c2dec10f68e48744ffe90f55 Clean up banning levels (Matt Corallo)
b8b4c80146780f9011abbd1be72343cc965c07b9 [refactor] drop IsInvalid(nDoSOut) (Matt Corallo)
8818729013e17c650a25f030b2b80e0997389155 [refactor] Refactor misbehavior ban decisions to MaybePunishNode() (Matt Corallo)
00e11e61c0211a62788611cd6a6714a393fdc26c [refactor] rename stateDummy -> orphan_state (Matt Corallo)
f34fa719cf33a51d11f1d2219cbe73ccff6fd697 Drop obsolete sigops comment (Matt Corallo)
Pull request description:
This is a rebase of #11639 with some fixes for the last few comments which were not yet addressed.
The original PR text, with some strikethroughs of text that is no longer correct:
> This cleans up an old main-carryover - it made sense that main could decide what DoS scores to assign things because the DoS scores were handled in a different part of main, but now validation is telling net_processing what DoS scores to assign to different things, which is utter nonsense. Instead, we replace CValidationState's nDoS and CorruptionPossible with a general ValidationInvalidReason, which net_processing can handle as it sees fit. I keep the behavior changes here to a minimum, but in the future we can utilize these changes for other smarter behavior, such as disconnecting/preferring to rotate outbound peers based on them providing things which are invalid due to SOFT_FORK because we shouldn't ban for such cases.
>
> This is somewhat complementary with, though obviously conflicts heavily with #11523, which added enums in place of DoS scores, as well as a few other cleanups (which are still relevant).
>
> Compared with previous bans, the following changes are made:
>
> Txn with empty vin/vout or null prevouts move from 10 DoS
> points to 100.
> Loose transactions with a dependency loop now result in a ban
> instead of 10 DoS points.
> ~~BIP68-violation no longer results in a ban as it is SOFT_FORK.~~
> ~~Non-SegWit SigOp violation no longer results in a ban as it
> considers P2SH sigops and is thus SOFT_FORK.~~
> ~~Any script violation in a block no longer results in a ban as
> it may be the result of a SOFT_FORK. This should likely be
> fixed in the future by differentiating between them.~~
> Proof of work failure moves from 50 DoS points to a ban.
> Blocks with timestamps under MTP now result in a ban, blocks
> too far in the future continue to not result in a ban.
> Inclusion of non-final transactions in a block now results in a
> ban instead of 10 DoS points.
Note: The change to ban all peers for consensus violations is actually NOT the change I'd like to make -- I'd prefer to only ban outbound peers in those situations. The current behavior is a bit of a mess, however, and so in the interests of advancing this PR I tried to keep the changes to a minimum. I plan to revisit the behavior in a followup PR.
EDIT: One reviewer suggested I add some additional context for this PR:
> The goal of this work was to make net_processing aware of the actual reasons for validation failures, rather than just deal with opaque numbers instructing it to do something.
>
> In the future, I'd like to make it so that we use more context to decide how to punish a peer. One example is to differentiate inbound and outbound peer misbehaviors. Another potential example is if we'd treat RECENT_CONSENSUS_CHANGE failures differently (ie after the next consensus change is implemented), and perhaps again we'd want to treat some peers differently than others.
ACKs for commit 0ff1c2:
jnewbery:
utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b
ryanofsky:
utACK 0ff1c2a838da9e8dc7f77609adc89124bbea3e2b. Only change is dropping the first commit (f3883a321bf4ab289edcd9754b12cae3a648b175), and dropping the temporary `assert(level == GetDoS())` that was in 35ee77f2832eaffce30042e00785c310c5540cdc (now c8b0d22698385f91215ce8145631e3d5826dc977)
Tree-SHA512: e915a411100876398af5463d0a885920e44d473467bb6af991ef2e8f2681db6c1209bb60f848bd154be72d460f039b5653df20a6840352c5f7ea5486d9f777a3
|
|
|
|
Though at the moment ChainActive() simply references `g_chainstate.m_chain`,
doing this change now clears the way for multiple chainstate usage and allows
us to script the diff.
-BEGIN VERIFY SCRIPT-
git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g'
-END VERIFY SCRIPT-
|
|
in preparation for the following scripted-diff commit.
|
|
This can't be a scripted-diff due to the confusion of the global
chainActive and the CChainState member of the same name.
This specific rename makes the following chainActive -> ::ChainActive() diff
scriptable.
|
|
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\.DoS(\(.*\), REJECT_\(.*\), \(true\|false\)/.DoS(\1, REJECT_\2/' src/validation.cpp src/consensus/tx_verify.cpp src/consensus/tx_check.cpp
sed -i 's/state.GetRejectCode(), state.GetRejectReason(), [^,]\+, state.GetDebugMessage())/state.GetRejectCode(), state.GetRejectReason(), state.GetDebugMessage())/' src/validation.cpp
sed -i 's/\.DoS([^,]*, /.Invalid\(/' src/validation.cpp src/consensus/tx_verify.cpp src/consensus/tx_check.cpp
-END VERIFY SCRIPT-
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
|
|
|
|
Co-authored-by: Anthony Towns <aj@erisian.com.au>
|
|
This is a first step towards cleaning up our DoS interface - make
validation return *why* something is invalid, and let net_processing
figure out what that implies in terms of banning/disconnection/etc.
Behavior change: peers will now be banned for providing blocks
with premature coinbase spends.
Co-authored-by: Anthony Towns <aj@erisian.com.au>
Suhas Daftuar <sdaftuar@gmail.com>
|
|
This eliminates a discrepancy between block validation with multiple
script check threads, versus a single script check thread.
|
|
Compared with previous bans, the following changes are made:
* Txn with empty vin/vout or null prevouts move from 10 DoS
points to 100.
* Loose transactions with a dependency loop now result in a ban
instead of 10 DoS points.
* Many pre-segwit soft-fork errors now result in a ban.
Note: Transactions that violate soft-fork script flags since P2SH do not generally
result in a ban. Also, banning behavior for invalid blocks is dependent on
whether the node is validating with multiple script check threads, due to a long-
standing bug. That inconsistency is still present after this commit.
* Proof of work failure moves from 50 DoS points to a ban.
* Blocks with timestamps under MTP now result in a ban, blocks
too far in the future continue to *not* result in a ban.
* Inclusion of non-final transactions in a block now results in a
ban instead of 10 DoS points.
Co-authored-by: Anthony Towns <aj@erisian.com.au>
|
|
This comment was confusing and incorrect when first added ("invalid rather than
merely non-standard" has the opposite meaning of what is actually the case),
and was also not updated after segwit with the correct variable names.
Delete it since the code reads just fine on its own.
Co-authored by: Anthony Towns <aj@erisian.com.au>
Suhas Daftuar <sdaftuar@gmail.com>
|
|
effe81f750 Move g_is_mempool_loaded into CTxMemPool::m_is_loaded (Ben Woosley)
bb8ae2c419 rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json (Ben Woosley)
Pull request description:
And use it to fix a race condition in mempool_persist.py:
https://travis-ci.org/Empact/bitcoin/jobs/487577243
Since e.g. getrawmempool returns errors based on this status, this
enables users to test it for readiness.
Fixes #12863
ACKs for commit effe81:
MarcoFalke:
utACK effe81f750
jnewbery:
utACK effe81f7503d2ca3c88cfdea687f9f997f353e0d
Tree-SHA512: 74328b0c17a97efb8a000d4ee49b9a673c2b6dde7ea30c43a6a2eff961a233351c9471f9a42344412135786c02bdf2ee1b2526651bb8fed68bd94d2120c4ef86
|
|
8722e54e56 threads: add thread names to deadlock debugging message (James O'Beirne)
383b186c28 threads: prefix log messages with thread names (James O'Beirne)
ddd95ccb80 tests: add threadutil tests (James O'Beirne)
ae5f2b6a6c threads: introduce util/threadnames, refactor thread naming (James O'Beirne)
188ca75e5f disable HAVE_THREAD_LOCAL on unreliable platforms (James O'Beirne)
Pull request description:
I'm resurrecting this one (from #13168) because I need it to make progress on #15735.
It's now off by default and can be turned on with `-logthreadnames=1`.
Ran some benchmarks (IBD from local peer from 500_000 -> 504_000) and it's within spitting distance either on or off:
### threadnames off (default)
#### 2018-05-threadnames.3 vs. master (absolute)
| name | iterations | 2018-05-threadnames.3 | master |
|------------------------------------------------|-----------:|----------------------------|----------------------------|
| ibd.local.500000.504000.dbcache=2048 | 3 | 376.1584 (± 9.2944) | 392.3414 (± 13.4238) |
| ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2236117.3333 (± 1845.9623) | 2238690.6667 (± 2669.3487) |
#### 2018-05-threadnames.3 vs. master (relative)
| name | iterations | 2018-05-threadnames.3 | master |
|------------------------------------------------|-----------:|----------------------:|-------:|
| ibd.local.500000.504000.dbcache=2048 | 3 | 1 | 1.043 |
| ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1 | 1.001 |
### threadnames on
#### 2018-05-threadnames-take-2 vs. master (absolute)
| name | iterations | 2018-05-threadnames-take-2 | master |
|------------------------------------------------|-----------:|----------------------------|----------------------------|
| ibd.local.500000.504000.dbcache=2048 | 3 | 367.6861 (± 0.3941) | 364.1667 (± 0.9776) |
| ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 2238461.3333 (± 3697.8730) | 2237014.6667 (± 3307.6966) |
#### 2018-05-threadnames-take-2 vs. master (relative)
| name | iterations | 2018-05-threadnames-take-2 | master |
|------------------------------------------------|-----------:|---------------------------:|-------:|
| ibd.local.500000.504000.dbcache=2048 | 3 | 1.010 | 1.00 |
| ibd.local.500000.504000.dbcache=2048.mem-usage | 3 | 1.001 | 1.00 |
```
ACKs for commit 8722e5:
Empact:
utACK https://github.com/bitcoin/bitcoin/pull/15849/commits/8722e54e56fd959fd4ff2321b36a7640dee440c5
jnewbery:
utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5
MarcoFalke:
re-utACK 8722e54e56fd959fd4ff2321b36a7640dee440c5 (Only change since my previous review is DEFAULT_LOGTHREADNAMES=false and stylistic updates
Tree-SHA512: 50af992708295b8d680cf10025262dd964e599a356bdfc1dfc84fb18c00afabcb34d3d12d551b0677ff81f8fccad0e17c1d5b24dfecb953a913bc77fdd1a4577
|
|
This work is prerequisite to attaching thread names to log lines and deadlock
debug utilities. This code allows setting of an "internal" threadname per
thread on platforms where thread_local is available.
This commit also moves RenameThread() out of a more general module and adds a
numeric suffix to disambiguate between threads with the same name. It
explicitly names a few main threads using the new util::ThreadRename().
|
|
5c759c73b2 [wallet] Move maxTxFee to wallet (John Newbery)
Pull request description:
Closes #15355
Moves the `-maxtxfee` from the node to the wallet. See discussion in issue for details.
This is a cleanup. There is no change in behaviour.
Completes #15620
ACKs for commit 5c759c:
MarcoFalke:
utACK 5c759c73b2602c7fde1c50dbafe5525904c1b64c
ryanofsky:
utACK 5c759c73b2602c7fde1c50dbafe5525904c1b64c. Changes since last review: updated commit message and an error message and method name.
meshcollider:
utACK https://github.com/bitcoin/bitcoin/pull/15778/commits/5c759c73b2602c7fde1c50dbafe5525904c1b64c
Tree-SHA512: 2f9b2729da3940a5cda994d3f3bc11ee1a52fcc1c5e9842ea0ea63e4eb0300e8416853046776311298bc449ba07554aa46f0f245ce28598a5b0bd7347c12e752
|
|
5d26205272 doc: explain AcceptToMemoryPoolWorker's coins_to_uncache (James O'Beirne)
Pull request description:
I found ATMPW's `coins_to_uncache` a little hard to understand (see #15264). This adds some doc for posterity.
ACKs for commit 5d2620:
jnewbery:
ACK 5d262052728acdaa2d108a35ba9921a23b3d761a
Tree-SHA512: 088508fa78012fab8680663c4e30f5cee29768416c2ca8b8b2abc29b6ac7067c5a589674f0254474a7ccc95477889d41719760f5796792bf492f51b3dd499c6c
|
|
|
|
418d3230f8 Resolve the checkpoints <-> validation CD. (251)
Pull request description:
This pull request attempts to resolve the `checkpoints -> validation -> checkpoints` circular dependency.
The circular dependency is resolved by moving the `CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` function to `validation.cpp` where it used exclusively by the private function `ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& params, const CBlockIndex* pindexPrev, int64_t nAdjustedTime)`.
ACKs for commit 418d32:
promag:
utACK 418d323, only `GetLastCheckpoint` usage is in `validation.cpp` and so makes sense to move it there.
practicalswift:
utACK 418d3230f86f77dde6e817f502baff8a54b707fa
MarcoFalke:
utACK 418d3230f86f77dde6e817f502baff8a54b707fa
sipa:
utACK 418d3230f86f77dde6e817f502baff8a54b707fa
Tree-SHA512: 03c3556bc192e65f5e3fa76fd545d4ee7d63d3fb06b132f7a1fa6131aa21ddd2e5b2d19e2222dfe524f422daaca30efde219bed188db8c74ff4b088876b5bc16
|
|
This commit moves the maxtxfee setting to the wallet. There is only
one minor behavior change:
- an error message in feebumper now refers to -maxtxfee instead of
maxTxFee.
|
|
4d074e84a2cf419510e2920417799f62747f4b07 [build] Move AnalyzePSBT from psbt.cpp to node/psbt.cpp (Russell Yanofsky)
fd509bd1f71df628b933ea7a135a9a957a5e0136 [docs] Document src subdirectories and different libraries (John Newbery)
9eaeb7fb8d4ab0d4493849e6c17e314fd75fea9c [build] Move wallet load functions to wallet/load unit (John Newbery)
91a25d1e711bfc0617027eee18b9777ff368d6b9 [build] Add several util units (John Newbery)
99517866b62c261f990e1f897502855afc12f2a7 [build] Move several units into common libraries (John Newbery)
0509465542d63a5bbe7296f283f44dd491e74f78 [build] Move rpc rawtransaction util functions to rpc/rawtransaction_util.cpp (John Newbery)
1acc61f8746bc6efb905e121a9f607c4f5982b35 [build] Move rpc utility methods to rpc/util (John Newbery)
4a75c9d6512a5580e60104103ea11d2cd9586354 [build] Move policy settings to new src/policy/settings unit (John Newbery)
fdf8888b6f0c63e8a4cb1459752625e642d6a4dd [build] Move CheckTransaction from lib_server to lib_consensus (John Newbery)
Pull request description:
This is a move-only commit. No code is changing and the moves can be easily verified with:
```sh
git log -p -n1 --color-moved=dimmed_zebra
```
This commit moves functions and variables that wallet code depends on out of libbitcoin_server.a, so the bitcoin-wallet tool can be built without libbitcoin_server.a in #15639, and attempting to access server state from wallet code will result in link errors instead of silently broken code.
List of moves:
- `CheckTransaction` moves from `consensus/tx_verify.cpp` to `consensus/tx_check.cpp`
- `urlDecode` moves from `httpserver.cpp` to `util/url.cpp`
- `TransactionErrorString` moves from `node/transaction.cpp` to `util/error.cpp`
- `StringForFeeReason` and `FeeModeFromString` move from `policy/fees.cpp` to `util/fees.cpp`
- `incrementalRelayFee` `dustRelayFee` and `nBytesPerSigOp` move from `policy/policy.cpp` to `policy/settings.cpp`
- `SignalsOptInRBF` moves from `policy/rbf.cpp` to `util/rbf.cpp`
- `fIsBareMultisigStd` moves from `validation.cpp` to `policy/settings.cpp`
- `ConstructTransaction` `TxInErrorToJSON` and `SignTransaction` move from `rpc/rawtransaction.cpp` to `rpc/rawtransaction_util.cpp`
- `RPCTypeCheck` `RPCTypeCheckArgument` `RPCTypeCheckObj` `AmountFromValue` `ParseHashV``ParseHashO` `ParseHexV` `ParseHexO` `HelpExampleCli` and `HelpExampleRpc` move from `rpc/server.cpp` to `rpc/util.cpp`
- `AmountHighWarn` and `AmountErrMsg` move from `ui_interface.cpp` to `util/error.cpp`
- `FormatStateMessage` and `strMessageMagic` move from `validation.cpp` to `util/validation.cpp`
- `VerifyWallets` `LoadWallets` `StartWallets` `FlushWallets` `StopWallets` and `UnloadWallets` move from `wallet/init.cpp` to `wallet/node.cpp`
ACKs for commit 4d074e:
jnewbery:
utACK 4d074e84a2cf419510e2920417799f62747f4b07 (checked by doing the rebase myself and verifying no difference between my branch and 4d074e84a2cf419510e2920417799f62747f4b07)
Tree-SHA512: 5e1604a9fb06475f2b96da0de0baa8330f4dda834dc20a0183ef11e1e4c27631d1d1bbb9abf0054efc03d56945fdf9920f63366b6a4f200f665b742a479ff75c
|
|
fae38c3dc6 doc: Fix all typos reported by codespell (MarcoFalke)
fa9058f0ed doc: Add release notes for 15629 (MarcoFalke)
fa4a922d78 qa: Add test for missing testnet section in conf file (MarcoFalke)
dddd6f0f58 init: Throw error when network specific config is ignored (MarcoFalke)
Pull request description:
This should have no effect on mainnet users, but simplifies testing, where config settings are currently ignored with only a warning. Fix this by making it an error.
Issues:
* bitcoin client 0.17.0 ignores wallet's name (file) #14523
* Can't set custom rpcport on testnet #13777
* ...
ACKs for commit fae38c:
Tree-SHA512: 2e209526898eea6e444c803ec2666989cee4ca137492d32984998733c50a70056cb54657df8dc3027a6a0612738a8afce0bc35824b868c5f22281e00e0188530
|
|
Adds the following util units and adds them to libbitcoin_util:
- `util/url.cpp` takes `urlDecode` from `httpserver.cpp`
- `util/error.cpp` takes `TransactionErrorString` from
`node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from
`ui_interface.cpp`
- `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp`
- `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp`
- 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
|
|
This moves the following policy settings functions and globals to a new
src/policy/settings unit in lib_server:
- `incrementalRelayFee`
- `dustRelayFee`
- `nBytesPerSigOp`
- `fIsBareMultisigStd`
These settings are only required by the node and should not be accessed
by other libraries.
|
|
CheckTransaction is a context-free function that does not require access
to the blockchain or mempool. Move it from src/consensus/tx_verify in
lib_server to a new unit src/consensus/tx_check in lib_consensus so that
it can be called by non-server libraries.
|
|
This commit resolves the checkpoints -> validation -> checkpoints
cirular dependency by moving
`CheckPoints::GetLastCheckpoint(const CCheckpointData& data)` from
`checkpoints.cpp` to `validation.cpp`.
|
|
|
|
So the loaded state is explicitly mempool-specific.
|