Age | Commit message (Collapse) | Author |
|
Delay initializing the TxRelay data structure for a peer until we receive
a version message from that peer. At that point we'll know whether it
will ever relay transactions. We only initialize the m_tx_relay
data structure if:
- this isn't an outbound block-relay-only connection; AND
- fRelay=true OR we're offering NODE_BLOOM to this peer
(NODE_BLOOM means that the peer may turn on tx relay later)
|
|
|
|
This fully comments all the TxRelay members. The only significant change
is to the comment for m_relay_txs. Previously the comment stated that
one of the purposes of the field was that "We don't relay tx invs before
receiving the peer's version message". However, even without the
m_relay_txs flag, we would not send transactions to the peer before
receiving the `version` message, since SendMessages() returns
immediately if fSuccessfullyConnected is not set to true, which only
happens once a `version` and `verack` message have been received.
|
|
Move m_next_send_feefilter and m_fee_filter_sent out of the `TxRelay`
data structure. All of the other members of `TxRelay` are related to
sending transactions _to_ the peer, whereas m_fee_filter_sent and
m_next_send_feefilter are both related to receiving transactions _from_
the peer. A node's tx relay behaviour is not always symmetrical (eg a
blocksonly node will ignore incoming transactions, but may still send
out its own transactions), so it doesn't make sense to group the
feefilter sending data with the TxRelay data in a single structure.
This does not change behaviour, since IsBlockOnlyConn() is always equal
to !peer.m_tx_relay. We still don't send feefilter messages to outbound
block-relay-only peers (tested in p2p_feefilter.py).
|
|
f403531f97cb80930e9a8e70a6f0dbde4cc8a866 Squashed 'src/univalue/' changes from a44caf65fe..6c19d050a9 (MacroFake)
Pull request description:
Only change is some header-shuffling and adding `getInt`.
ACKs for top commit:
fanquake:
ACK fac2c796cb4137e025a4a783f7460e5db9c74bc2
Tree-SHA512: 8bdf7d88ce06f0851f99f30c30fc926a13b79ae72bcebd5e170ed0e1d882b184d9279f96488e234fbe560036e31cb180aa1e5666aebd9833b9a119c6b214fa30
|
|
global to ChainstateManager
bb5c24b120a3ac7df367a1c5d9b075ca564efb5f validation: move g_versionbitscache into ChainstateManager (Anthony Towns)
eca22c726ac48b4216bb68cc0f0bbd655c43ac12 test/versionbits: make versionbitscache a parameter (Anthony Towns)
d603f1d8a7cdc0a158ed80ade8a843b61b6ad08e deploymentstatus: make versionbitscache a parameter (Anthony Towns)
78adef17536edef833a0bfca06b61ce28120e486 refactor: use chainman instead of chainParams for DeploymentActive* (Anthony Towns)
deffe0df6c36225bada18603b5a840139f030f2c deploymentstatus: allow chainman in place of consensusParams (Anthony Towns)
eaa2e3f25cefbd1b9a1214102f88dbfa8109d244 validation: move UpdateUncommittedBlockStructures and GenerateCoinbaseCommitment into ChainstateManager (Anthony Towns)
5c67e84d37d452e9186a6357e5405fabeff241c7 validation: replace ::Params() calls with chainstate/chainman member (Anthony Towns)
38860f93b680f152fc6fc3d9ae574a4c0659e775 validation: remove redundant CChainParams params from ChainstateManager methods (Anthony Towns)
69675ea4e73dcf5e9dd0f94802bd3463e4262081 validation: add CChainParams to ChainstateManager (Anthony Towns)
Pull request description:
Gives `ChainstateManager` a reference to the `CChainParams` its working on, and simplifies some of the functions that would otherwise take that as a parameter. Removes the `g_versionbitscache` global by moving it into `ChainstateManager`.
ACKs for top commit:
dongcarl:
reACK bb5c24b120a3ac7df367a1c5d9b075ca564efb5f
MarcoFalke:
review ACK bb5c24b120a3ac7df367a1c5d9b075ca564efb5f π
Tree-SHA512: 3fa74905e5df561e3e74bb0b8fce6085c5311e6633e7d74c0fb0c82a907f5bbb1fd4ebc5d11d4f0b1c019bb51eabb9f6e4bcc4652a696d36a5878c807b85f121
|
|
from header to implementation
51ec96b904f349056e805c6b2a6de5257e8fbdee refactor: move StartExtraBlockRelayPeers from header to implementation (Jon Atack)
Pull request description:
where all the other logging actions in src/net.{h,cpp} are located.
StartExtraBlockRelayPeers() does not appear to be a hotspot that needs to be inlined for performance, as it is called from CheckForStaleTipAndEvictPeers(), called in turn from StartScheduledTasks() with a scheduleEvery delta of 45 seconds, called at the end of AppInitMain() on bitcoind startup.
This allows dropping `#include <logging.h>` from net.h, which can improve compile time/speed. Currently, none of the other includes in net.h use logging.h, except src/sync.h if DEBUG_LOCKCONTENTION is defined.
ACKs for top commit:
LarryRuane:
ACK 51ec96b904f349056e805c6b2a6de5257e8fbdee
theStack:
ACK 51ec96b904f349056e805c6b2a6de5257e8fbdee
Tree-SHA512: 69b2c09163c48bfcb43355af0aa52ee7dd81efc755a7aa6a10f5e400b5e14109484437960a62a1cfac2524c2cfae981fee082846b19526b540ef5b86be97f0fe
|
|
where all the other logging actions in src/net.{h,cpp} are located.
StartExtraBlockRelayPeers() does not appear to be a hotspot that needs to be
inlined for performance, as it is called from CheckForStaleTipAndEvictPeers(),
called in turn from StartScheduledTasks() with a scheduleEvery delta of 45
seconds, called at the end of AppInitMain() on bitcoind startup.
This allows dropping `#include <logging.h>` from net.h, which can improve
compile time/speed. Currently, none of the other includes in net.h use
logging.h, except src/sync.h if DEBUG_LOCKCONTENTION is defined.
|
|
-> `uint`...)
-BEGIN VERIFY SCRIPT-
sed -i 's/u_int/uint/g' $(git grep -l u_int)
-END VERIFY SCRIPT-
|
|
|
|
fab9e8a29c2cdeab6cf1ae7c1fc0e0a3af783b17 Remove unused GetTimeSeconds (MacroFake)
Pull request description:
Seems confusing to have this helper when it is possible to get the system time in a type-safe way by simply calling `std::chrono::system_clock::now` (C++11).
This patch replaces `GetTimeSeconds` and removes it:
* in `bitcoin-cli.cpp` by `system_clock`
* in `test/fuzz/fuzz.cpp` by `steady_clock`
ACKs for top commit:
laanwj:
Code review ACK fab9e8a29c2cdeab6cf1ae7c1fc0e0a3af783b17
naumenkogs:
ACK fab9e8a29c2cdeab6cf1ae7c1fc0e0a3af783b17
Tree-SHA512: 517e300b0baf271cfbeebd4a0838871acbea9360f9dd23572a751335c20c1ba261b1b5ee0aec8a36abd20c94fab83ce94f46042745279aca1f0ca2f885a03b6e
|
|
GetRandInt
ab1ea29ba1b8379a21fabd3dc859552c470a6421 refactor: make GetRand a template, remove GetRandInt (pasta)
Pull request description:
makes GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
This simplifies a lot of code from GetRand(std::numeric_limits<uint64_t>::max() -> GetRand<uint64_t>()
ACKs for top commit:
laanwj:
Code review ACK ab1ea29ba1b8379a21fabd3dc859552c470a6421
Tree-SHA512: db5082a0e21783389f1be898ae73e097b31ab48cab1a2c0e29348a4adeb545d4098193aa72a547c6baa6e8205699aafec38d6a27b3d65522fb3246f91b4daae9
|
|
fa9051642269f62f560af3f323fbf36cb7b58082 Switch scheduler to steady_clock (MacroFake)
Pull request description:
There is already `mockscheduler`, so it seems brittle, confusing and redundant to be able to mock the scheduler by adjusting the system clock.
ACKs for top commit:
laanwj:
Code review ACK fa9051642269f62f560af3f323fbf36cb7b58082
w0xlt:
crACK https://github.com/bitcoin/bitcoin/pull/25100/commits/fa9051642269f62f560af3f323fbf36cb7b58082
Tree-SHA512: 60e99065ffb881a9fb25a346d311d99424fbc72a3b636c94b5f5c17ed6373c40f358a9b27825c518d12968c033e6cfd3c62d2b62cacdddc44a0b5b74f6c1a7ae
|
|
bdc6881e2f796f4a9a5873826219e24f17a96a7c wallet: Change log interval to use `steady_clock` (w0xlt)
Pull request description:
This refactors the log interval variables to use `steady_clock` as it is best suitable for measuring intervals.
ACKs for top commit:
laanwj:
This makes sense. Code review ACK bdc6881e2f796f4a9a5873826219e24f17a96a7c
dunxen:
Code review ACK bdc6881
Tree-SHA512: 738b4aa45cef01df77102320f83096a0a7d0c63d7fcf098a8c0ab16b29453a87dc789c110105590e1e215d03499db1d889a94f336dcb385b6883c8364c9d39b7
|
|
|
|
This change improves the usability of the `dumptxoutset` RPC in two ways,
in the case that an invalid path is passed:
1. return from the RPC immediately, rather then when the file is first
tried to be written (which is _after_ calculating the UTXO set hash)
2. return a proper return code and error message instead of the cryptic
"CAutoFile::operator<<: file handle is nullptr: unspecified
iostream_category error" (-1)
|
|
This refactors the log interval variables to use `steady_clock`
as it is best suitable for measuring intervals.
|
|
|
|
and FastRandomContext::rand_uniform_delay
fa4fb8d98b7e8e5ea2db35bf239fa7f248da5d8e random: Add FastRandomContext::rand_uniform_delay (MarcoFalke)
faa5c62967174f1dd66e8a4ba61ab29c867cf450 Add time helpers for std::chrono::steady_clock (MarcoFalke)
Pull request description:
A steady clock can be used in the future for the scheduler, for example.
A random uniform delay applied to a time point can be used in the future for time points passed to the scheduler, or delays in net processing.
Currently they are unused outside of tests, but if they turn out unused in the future (unlikely), they can trivially be removed again. I am splitting them out, so that several branches/pulls can build on top of them without duplicating the commits.
ACKs for top commit:
ajtowns:
ACK fa4fb8d98b7e8e5ea2db35bf239fa7f248da5d8e
Tree-SHA512: 2c37174468fe84b1cdf2a032f458706df44b99a5f99062417bb42078b6f69e2f1738d20c21cd9386ca5a35f3bc0583e547ba40168c66f6aa42f700ba35dd95d4
|
|
`std::chrono::steady_clock`
92b35aba224ad4440f3ea6c01c841596a6a3d6f4 index, refactor: Change sync variables to use `std::chrono::steady_clock` (w0xlt)
Pull request description:
This PR refactors the sync variables to use `std::chrono::steady_clock` as it is best suitable for measuring intervals.
ACKs for top commit:
jonatack:
utACK 92b35aba224ad4440f3ea6c01c841596a6a3d6f4
ajtowns:
ACK 92b35aba224ad4440f3ea6c01c841596a6a3d6f4 - code review only
Tree-SHA512: cd4bafde47b30beb88c0aac247e41b4dced2ff2845c67a7043619da058dcff4f84374a7c704a698f3055c888d076d25503c2f38ace8fbc5456f624e0efe1e188
|
|
|
|
|
|
|
|
|
|
|
|
GenerateCoinbaseCommitment into ChainstateManager
|
|
|
|
|
|
|
|
with `const`
f70ee34c71aeeb814fe65a69952343dccdb7b906 qt, refactor: Declare `WalletModel` member functions with `const` (Hennadii Stepanov)
Pull request description:
After bitcoin/bitcoin#12830 the `WalletModel` class has two member functions: https://github.com/bitcoin-core/gui/blob/be7a5f2fc400e7a3ef72dedbdcf49dd6c96d4f9e/src/qt/walletmodel.h#L81 and https://github.com/bitcoin-core/gui/blob/be7a5f2fc400e7a3ef72dedbdcf49dd6c96d4f9e/src/qt/walletmodel.h#L154
This PR drops the former one as redundant, and declares `WalletModel` member functions with the `const` qualifier where appropriate.
ACKs for top commit:
promag:
Code review ACK f70ee34c71aeeb814fe65a69952343dccdb7b906.
kristapsk:
cr ACK f70ee34c71aeeb814fe65a69952343dccdb7b906
w0xlt:
Code Review ACK https://github.com/bitcoin-core/gui/pull/590/commits/f70ee34c71aeeb814fe65a69952343dccdb7b906
Tree-SHA512: 43e6661822c667229ea860fb94c2e3154c33773dbd9fca1f6f76cc31c5875a1a0e8caa65ddfc20dec2a43e29e7b2469b3b6fa148fe7ec000ded518b4958b2b38
|
|
`AddressBookPage` dialog
15069130c6ca5273f3a593a404f60f11caa7d950 qt, test: Add tests for `tableView` in `AddressBookPage` dialog (Hennadii Stepanov)
edae3ab6999ee9e6efabd8403d31e9bd7c84f8a3 qt: No need to force Qt::QueuedConnection for NotifyAddressBookChanged (Hennadii Stepanov)
Pull request description:
This PR is a prerequisite for more thorough testing of filtering in the `AddressBookPage` class in context of bitcoin-core/gui#578 and bitcoin-core/gui#585.
Required for bitcoin-core/gui#592.
ACKs for top commit:
promag:
Code review ACK 15069130c6ca5273f3a593a404f60f11caa7d950.
Tree-SHA512: 86986d47606cbd54d813436c7afb21894e2200b6d3042a7aa0b5e84821c765bd68b14ad38a445069891ab33f2d7bcd4933b8373e14e9afb0c91f1a6ddf4da740
|
|
81c09ee45caecf8d9daf6766b94cebf54f3f08cd Unroll the ChaCha20 inner loop for performance (Pieter Wuille)
Pull request description:
Unrolling the inner ChaCha20 loop gives a ~15% speedup for me in the CHACHA20_* benchmarks. It's a simple change, this performance helps with RNG generation, and will matter more for BIP324.
ACKs for top commit:
martinus:
tested ACK 81c09ee with clang++ 13.0.1, test `CHACHA20_1MB`:
MarcoFalke:
ACK 81c09ee45caecf8d9daf6766b94cebf54f3f08cd π
Tree-SHA512: 108bd0ba573bb08de92d611e7be7c09a2c2700f9655f44129b87f9b71f7e101dfc6bd345783e7b4b9b40f0b003913cf59187f422da8cdb5b20887f7855b2611a
|
|
|
|
|
|
This refactors the sync variables to use `std::chrono::steady_clock`
as it is best suitable for measuring intervals.
|
|
appropriate
308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Sanity assert GetAncestor() != nullptr where appropriate (Adam Jonas)
Pull request description:
Re-opening #17232. I have rebased the PR and addressed jonatack's nit suggestions.
Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate.
In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time.
In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior.
Co-Authored-By: Adam Jonas <jonas@chaincode.com>
Co-Authored-By: danra <danra@users.noreply.github.com>
ACKs for top commit:
jonatack:
ACK 308dd2e93e92f4cac4e7d75478316af9bb2b77b8
Tree-SHA512: 5bfdaab1499607ae2c3cd3e2e9e8c37850bfd0e327e680f4e36c81f9c6d98a543af78ecfac1ab0e06325d264412615a04d52005875780c7db2a4d81bd2d2259a
|
|
MutableTransactionSignatureCreator
fac6cfc50f65c610f2df9af3ec2efff5eade6661 refactor: Change * to & in MutableTransactionSignatureCreator (MarcoFalke)
Pull request description:
The `MutableTransactionSignatureCreator` constructor takes in a pointer to a mutable transaction. This is problematic for several reasons:
* It would be undefined behaviour to pass in a nullptr because for signature creation, the memory of the mutable transaction is accessed
* No caller currently passes in a nullptr, so passing a reference as a pointer is confusing
Fix all issues by replacing `*` with `&` in `MutableTransactionSignatureCreator`
ACKs for top commit:
theStack:
Code-review ACK fac6cfc50f65c610f2df9af3ec2efff5eade6661
jonatack:
ACK fac6cfc50f65c610f2df9af3ec2efff5eade6661
Tree-SHA512: d84296b030bd4fa2709e5adbfe43a5f8377d218957d844af69a819893252af671df7f00004f5ba601a0bd70f3c1c2e58c4f00e75684da663f28432bb5c89fb86
|
|
using modified fees, not base
e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb [unit test] prioritisation in mining (glozow)
7a8d60676bc0eec289687b2dfd5d2b00b83c0eaa [miner] bug fix: update for parent inclusion using modified fee (glozow)
0f9a44461c294cf21a335e8a8c13e498baac110f MOVEONLY: group miner tests into MinerTestingSetup functions (glozow)
Pull request description:
Came up while reviewing #24364, where some of us incorrectly assumed that we use the same fee deduction in `CTxMemPoolModifiedEntry::nModFeesWithAncestors` when first constructing an entry and in `update_for_parent_inclusion`.
Actually, the behavior is this: when a mempool entry's ancestor is included in the block template, we create a `CTxMemPoolModifiedEntry` for it, subtracting the ancestor's modified fees from `nModFeesWithAncestors`. If another ancestor is included, we update it again, but use the ancestor's _base_ fees instead.
I can't explain why we use `GetFee` in one place and `GetModifiedFee` in the other, but I'm quite certain we should be using the same one for both.
And should it be base or modified fees? Modified, otherwise the child inherits the prioritisation of the parent, but only until the parent gets mined. If we want prioritisation to cascade down to current in-mempool descendants, we should probably document that in the `prioritsetransaction` helpstring and implement it in `CTxMemPool::mapDeltas`, not as a quirk in the mining code?
Wrote a test in which a mempool entry has 2 ancestors, both prioritised, and both included in a block template individually. This test should fail without the s/GetFee/GetModifiedFee commit.
ACKs for top commit:
ccdle12:
tested ACK e4303c3
MarcoFalke:
ACK e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb π
Tree-SHA512: 4cd94106fbc9353e9f9b6d5af268ecda5aec7539245298c940ca220606dd0737264505bfaae1f83d94765cc2d9e1a6e913a765048fe6c19292482241761a6762
|
|
|
|
Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate.
In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time.
In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior.
Co-Authored-By: Aurèle Oulès <aurele@oules.com>
Co-Authored-By: danra <danra@users.noreply.github.com>
|
|
and src/rpc/net.cpp
e71c51b27d420fbd6cc0a36f62e63e190e13473a refactor: rename command -> message type in comments in the src/net* files (Shashwat)
2b09593bddb0a93aebf84e5f43cdb4d5282c7984 scripted-diff: Rename message command to message type (Shashwat)
Pull request description:
This PR is a follow-up to #24078.
> a message is not a command, but simply a message of some type
The first commit covers the message_command variable name and comments not addressed in the original PR in `src/net*` files.
The second commit goes beyond the original `src/net*` limit of #24078 and does similar changes in the `src/rpc/net.cpp` file.
ACKs for top commit:
MarcoFalke:
review ACK e71c51b27d420fbd6cc0a36f62e63e190e13473a π₯
Tree-SHA512: 24015d132c00f15239e5d3dc7aedae904ae3103a90920bb09e984ff57723402763f697d886322f78e42a0cb46808cb6bc9d4905561dc6ddee9961168f8324b05
|
|
b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93 doc: update init.cpp -conf help text (josibake)
970b9987ad5bcb72e581c40a7cdd408d94a48c81 doc: update devtools, release-process readmes (josibake)
50635d27b45d125b6264ac2abfbd6a1129c7228f build: include bitcoin.conf in build outputs (josibake)
6aac946f49aea243de1dc50631bb72f0186bbf58 doc: update bitcoin-conf.md (Josiah Baker)
1c7e820ded0846ef6ab4be9616b0de452336ef64 script: add script to generate example bitcoin.conf (josibake)
b483084d866c16d97a34251ae652bac94f85f61d doc: replace bitcoin.conf with placeholder file (josibake)
Pull request description:
create a script for parsing the output from `bitcoind --help` to create an example conf file for new users
## problem
per #10746 , `bitcoin.conf` not being put into the data directory during installation causes some confusion for users when running bitcoin. in the discussion on the issue, one proposed solution was to have an example config file and instruct users to `cp` it into their data directory after startup. in addition to #10746 , there have been other requests for a "skeleton config file" (https://github.com/bitcoin/bitcoin/issues/19641) to help users get started with configuring bitcoind.
the main issue with an example config file is that it creates a second source of truth regarding what options are available for configuring bitcoind. this means any changes to the options (including the addition or removal of options) would have to be updated for the command line and also updated in the example file.
this PR addresses this issue by providing a script to generate an example file directly from the `bitcoind --help` on-demand by running `contrib/devtools/gen-bitcoin-conf.sh`. this solution was originally proposed on #10746 and would also solve #19641 . this guarantees any changes made to the command-line options or the command-line options help would also be reflected in the example file after compiling and running the script.
the main purpose of this script is to generate a config file to be included with releases, same as `gen-manpages.sh`. this ensures every release also includes an up-to-date, full example config file for users to edit. the script is also available for users who compile from source for generating an example config for their compiled binary.
## special considerations
this removes the `bitcoin.conf` example file from the repo as it is now generated by this script. the original example file did contain extra text related to how to use certain options but going forward all option help docs should be moved into `init.cpp`
this also edits `init.cpp` to have the option help indicate that `-conf` is not usable from the config file. this is similar to how `-includeconf` 's help indicates it cannot be used from the command line
ACKs for top commit:
laanwj:
Tested and code review ACK b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93
Tree-SHA512: 4546e0cef92aa1398da553294ce4712d02e616dd72dcbe0b921af474e54f24750464ec813661f1283802472d1e8774e634dd1cc26fbf1f13286d3e0406c02c09
|
|
e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c test: Add `strerror` to locale-dependence linter (laanwj)
f00fb1265a8bc26e1612c771173325dbe49b3612 util: Increase buffer size to 1024 in SysErrorString (laanwj)
718da302c7b11b375042c3000d421fd93348c199 util: Refactor SysErrorString logic (laanwj)
e7f2f77756d33c6be9c8998a575b263ff2d39270 util: Use strerror_s for SysErrorString on Windows (laanwj)
46971c6dbfbc39ebbc74ab1ed8c00edc12859373 util: Replace non-threadsafe strerror (laanwj)
Pull request description:
Some uses of non-threadsafe `strerror` have snuck into the code since they were removed in #4152. Add a wrapper `SysErrorString` for thread-safe strerror alternatives (with code from `NetworkErrorString`) and replace all uses of `strerror` with this.
Edit: I've also added a commit that refactors the code so that buf[] is never read at all if the function fails, making some fragile-looking code unnecessary.
Edit2: from the linux manpage:
```
ATTRIBUTES
For an explanation of the terms used in this section, see attributes(7).
βββββββββββββββββββββ¬ββββββββββββββββ¬ββββββββββββββββββββββββββ
βInterface β Attribute β Value β
βββββββββββββββββββββΌββββββββββββββββΌββββββββββββββββββββββββββ€
βstrerror() β Thread safety β MT-Unsafe race:strerror β
βββββββββββββββββββββΌββββββββββββββββΌββββββββββββββββββββββββββ€
β¦
βββββββββββββββββββββΌββββββββββββββββΌββββββββββββββββββββββββββ€
βstrerror_r(), β Thread safety β MT-Safe β
βstrerror_l() β β β
βββββββββββββββββββββ΄ββββββββββββββββ΄ββββββββββββββββββββββββββ
```
As the function can be called from any thread at any time, using a non-thread-safe function is unacceptable.
ACKs for top commit:
jonatack:
ACK e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c
Tree-SHA512: 20e71ebb9e979d4e1d8cafbb2e32e20c2a63f09115fe72cdde67c8f80ae98c531d286f935fd8a6e92a18b72607d7bd3e846b2d871d9691a6036b0676de8aaf25
|
|
|
|
update help to reflect this option cannot be used from the config file
|
|
5e61532e72c1021fda9c7b213bd9cf397cb3a802 util: optimizes HexStr (Martin Leitner-Ankerl)
4e2b99f72a90b956f3050095abed4949aff9b516 bench: Adds a benchmark for HexStr (Martin Leitner-Ankerl)
67c8411c37b483caa2fe3f7f4f40b68ed2a9bcf7 test: Adds a test for HexStr that checks all 256 bytes (Martin Leitner-Ankerl)
Pull request description:
In my benchmark, this rewrite improves runtime 27% (g++) to 46% (clang++) for the benchmark `HexStrBench`:
g++ 11.2.0
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 0.94 | 1,061,381,310.36 | 0.7% | 12.00 | 3.01 | 3.990 | 1.00 | 0.0% | 0.01 | `HexStrBench` master
| 0.68 | 1,465,366,544.25 | 1.7% | 6.00 | 2.16 | 2.778 | 1.00 | 0.0% | 0.01 | `HexStrBench` branch
clang++ 13.0.1
| ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 0.80 | 1,244,713,415.92 | 0.9% | 10.00 | 2.56 | 3.913 | 0.50 | 0.0% | 0.01 | `HexStrBench` master
| 0.43 | 2,324,188,940.72 | 0.2% | 4.00 | 1.37 | 2.914 | 0.25 | 0.0% | 0.01 | `HexStrBench` branch
Note that the idea for this change comes from denis2342 in #23364. This is a rewrite so no unaligned accesses occur.
Also, the lookup table is now calculated at compile time, which hopefully makes the code a bit easier to review.
ACKs for top commit:
laanwj:
Code review ACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802
aureleoules:
tACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802.
theStack:
ACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802 π€
Tree-SHA512: 40b53d5908332473ef24918d3a80ad1292b60566c02585fa548eb4c3189754971be5a70325f4968fce6d714df898b52d9357aba14d4753a8c70e6ffd273a2319
|
|
e5d183151709ab59d2fa6fe9e0243000e8d6abbe [netgroup] Use nStartByte as offset for the last byte of the group (dergoegge)
Pull request description:
This addresses my review [comments](https://github.com/bitcoin/bitcoin/pull/22910#discussion_r856095896) I left on #22910.
This has no effect on the current logic as `nStartByte` is only used for internal addresses which only ever add 10 whole bytes to the returned group. However to avoid future bugs, I think we should use `nStartByte` as offset for the last byte as well, in case we ever add a new address type that makes makes use of `nStartByte` and adds fractional bytes to the group.
ACKs for top commit:
jnewbery:
Code review ACK e5d183151709ab59d2fa6fe9e0243000e8d6abbe
theStack:
Concept and code-review ACK e5d183151709ab59d2fa6fe9e0243000e8d6abbe
Tree-SHA512: 4c08c7d6cb38b553e998798b3e3b790177aaa2141a48e277dfd538e01a7fccadf644329e93c5b0fb5e7e4037494c8dfe061b94eb52c6b31dc21bdf99eb0e311a
|
|
SplitString
f849e63bad963b8717d4bc45efdad9b08567a36e fuzz: SplitString with multiple separators (Martin Leitner-Ankerl)
d1a9850102fe572b8a1e00b80c757dd82bf39f9d http: replace boost::split with SplitString (Martin Leitner-Ankerl)
0d7efcdf75607e19fac77bcd146773a03af14492 core_read: Replace boost::split with SplitString (Martin Leitner-Ankerl)
b7ab9db545492927b774912e53aeb834a590621f Extend Split to work with multiple separators (Martin Leitner-Ankerl)
Pull request description:
As a followup of #22953, this removes the remaining occurrences of `boost::split` and replaces them with our own `SplitString`. To be able to do so, this extends the function `spanparsing::Split` to work with multiple separators. Finally this removes 3 more files from `lint-includes.py`.
ACKs for top commit:
theStack:
Code-review ACK f849e63bad963b8717d4bc45efdad9b08567a36e
Tree-SHA512: f37d4dbe11cab2046e646045b0f018a75f978d521443a2c5001512737a1370e22b09247d5db0e5c9e4153229a4e2d66731903c1bba3713711c4cae8cedcc775d
|
|
|
|
GetFirstStoredBlock()::start_time
4cb9d214345550cb0299139b2badb73ba1e53532 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack)
Pull request description:
Suggested in https://github.com/bitcoin/bitcoin/pull/25016#discussion_r862330288, the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time.
See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and #22278 for related discussion, and #25040 for a similar example.
ACKs for top commit:
MarcoFalke:
review ACK 4cb9d214345550cb0299139b2badb73ba1e53532
Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
|