e177fcab3831b6d259da5164cabedcc9e78f6957 Replace `struct update_lock_points` with lambda (glozow)
c7cd98c7176800a51e6a6b3634a26b508aa33ff2 document and clean up MaybeUpdateMempoolForReorg (glozow)
Pull request description:
followup to #23683, addressing https://github.com/bitcoin/bitcoin/pull/23683#issuecomment-989741186
ACKs for top commit:
hebasto:
ACK e177fcab3831b6d259da5164cabedcc9e78f6957, I have reviewed the code and it looks OK, I agree it can be merged.
instagibbs:
ACK e177fcab3831b6d259da5164cabedcc9e78f6957
MarcoFalke:
Approach ACK e177fcab3831b6d259da5164cabedcc9e78f6957 😶
Tree-SHA512: 8c2709dd5cab73cde41f3e5655d5f237bacfb341f78eac026169be579528695ca628c8777b7d89760d8677a4e6786913293681cfe16ab702b30c909703e1824c
bf044ef9ecc93a69619cbaa9fa2b874d5fc06932 build: specify hosts for qrencode package (fanquake)
Pull request description:
Similar to how we specify the OS's we build Qt for, specify which OS's
we will build qrencode for (a qt dependency). This commit alone doesn't
change anything, but when we start supporting other OS's, i.e #23948,
where we wont support qt (or at least initially), it'll skip building
the qrencode package, which would be unused.
I'll rebase the other *BSD changes on top of this.
ACKs for top commit:
hebasto:
ACK bf044ef9ecc93a69619cbaa9fa2b874d5fc06932, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3f5f11f122704a664dd77d8da0b7e9b95d44b2f1514d0199deed9b8b8ad0d8883a1de1f444b796c5f4681f423a380c3905fce720d7d2b788130162c907c2ce3b
-BEGIN VERIFY SCRIPT-
s() { sed -i 's/cs_mapLocalHost/g_maplocalhost_mutex/g' $1; }
s src/net.cpp
s src/net.h
s src/rpc/net.cpp
s src/test/net_tests.cpp
-END VERIFY SCRIPT-
bd52684508ca2964e6a3af503d21ff99675380c7 test: rest /tx with an invalid/unknown txid (brunoerg)
Pull request description:
This PR adds test coverage to the endpoint `/tx` (rest) passing an invalid and an unknown txid to test its return.
Invalid -> should return status code 400 (bad request)
Unknown -> should return status code 404 (not found)
ACKs for top commit:
kallewoof:
ACK bd52684508ca2964e6a3af503d21ff99675380c7
Tree-SHA512: a7fbb63f30d06fc0855133a36e8317c7930ba13aa2b4a2dd1fc35079d59eacace72e1ffe7ae1b3e067066fe51792415940d72d923e83a659a0d5965e4110b32a
86a4a15bdcc96eb565ab80166642d71d542061a9 Highlight DNS request part (Prayank)
Pull request description:
_What?_
Highlight DNS requests part in Proxy section
_Why?_
1. DNS requests are very important while considering privacy
2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now
3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools
_How?_
Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r618177116). Harding suggested using [bold/italic in `<pre></pre>`](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r644364143). I have used the suggestions from previous PR and added `---`
This is a part of alternative described in https://github.com/bitcoin/bitcoin/pull/22316
ACKs for top commit:
jonatack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Rspigler:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
achow101:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
RiccardoMasutti:
ACK 86a4a15
lsilva01:
ACK 86a4a15bdc
kristapsk:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
theStack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
No behavior change.
This code was introduced in 5add7a7 before we required C++11, which is
why the struct was needed. As we are now using more modern C++ and this
is the only place where lockpoints are updated for mempool entries, it
is more idiomatic to call `modify` with a lambda.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Similar to how we specify the OS's we build Qt for, specify which OS's
we will build qrencode for (a qt dependency). This commit alone doesn't
change anything, but when we start supporting other OS's, i.e #23948,
where we wont support qt (or at least initially), it'll skip building
the qrencode package, which would be unused.
Behavior change: don't quit right after LimitMempoolSize() when a
package is partially submitted. We should still send
TransactionAddedToMempool notifications for
transactions that were submitted.
Not behavior change: add a new package validation result for mempool logic errors.
The previous interface required callers to guess that the tx had been
swapped and look up the tx again by txid to find a `MEMPOOL_ENTRY`
result. This is a confusing interface.
Instead, explicitly tell the caller that this transaction was
`DIFFERENT_WITNESS` in the result linked to the mempool entry's wtxid.
This gives the caller all the information they need in 1 lookup, and
they can query the mempool for the other transaction if needed.
7f122a4188af7130be9251611e41136a17c814f1 fuzz: non-addrman fuzz tests: override-able check ratio (Vasil Dimov)
3bd83e273d104e9474af8f1bdf4f969163e33ade fuzz: addrman fuzz tests: override-able check ratio (Vasil Dimov)
46b0fe78298c8f416a91dec9d4e0f3f4cb1e68b0 test: non-addrman unit tests: override-able check ratio (Vasil Dimov)
81e4d54d3a95f7bffeb353217a6c32eb2aca8b5c test: addrman unit tests: override-able check ratio (Vasil Dimov)
6dff6214be768a3fab6d5201daf5ef6071764746 bench: put addrman check ratio in a variable (Vasil Dimov)
6f7c7567c578b5a41f8e90ce4491e40f7faeaa56 fuzz: parse the command line arguments in fuzz tests (Vasil Dimov)
92a0f7e58d4b6323d21f1c45d4c20266c35df030 test: parse the command line arguments in unit tests (Vasil Dimov)
Pull request description:
Previously command line arguments passed to unit and fuzz tests would be ignored by the tests themselves. They would be used by the boost test framework (e.g. `--run_test="addrman_tests/*"`) or by the fuzzer (e.g. `-runs=1`). However both provide ways to pass down the extra arguments to the test itself. Use that, parse the arguments and make them available to the tests via `gArgs`.
This makes the tests more flexible as they can be run with any bitcoind config option specified on the command line.
When creating `AddrMan` objects in tests, use `-checkaddrman=` (if provided) instead of hardcoding the check ratio in many different places. See https://github.com/bitcoin/bitcoin/pull/20233#issuecomment-889813074 for further motivation for this.
ACKs for top commit:
mzumsande:
re-ACK 7f122a4188af7130be9251611e41136a17c814f1
josibake:
reACK 7f122a4188
Tree-SHA512: 3a05e61e4d70a0569bb67594bcce3aad8fdef63cdcc54e2823a3bc9f18679571985004412b6c332a210f67849bab32d8467b4115fbff8f5fac9834982e60dcf3
5574e6ed52d6effd3b7beff0f09b44449202a585 refactor: replace RecursiveMutex `m_callbacks_mutex` with Mutex (Sebastian Falbesoner)
3aa258109e3f3e0b1bfc4f811cbadfd6d516208c scripted-diff: rename `m_cs_callbacks_pending` -> `m_callbacks_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `m_cs_callbacks_pending`. All of the critical sections (6 in total) only directly access the guarded elements, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex:
807169e10b/src/scheduler.cpp (L138-L145)807169e10b/src/scheduler.cpp (L152-L160)807169e10b/src/scheduler.cpp (L169-L172)807169e10b/src/scheduler.cpp (L184-L187)807169e10b/src/scheduler.cpp (L197-L199)807169e10b/src/scheduler.cpp (L203-L206)
Also, it is renamed to adapt to the (unwritten) naming convention to use the `_mutex` suffix rather than the `cs_` prefix.
ACKs for top commit:
hebasto:
ACK 5574e6ed52d6effd3b7beff0f09b44449202a585, I have reviewed the code and it looks OK, I agree it can be merged.
w0xlt:
crACK 5574e6e
Tree-SHA512: ba4b151d956582f4c7183a1d51702b269158fc5e2902c51e6a242aaeb1c72cfcdf398f9ffa42e3072f5aba21a8c493086a5fe7c450c271322da69bd54c37ed1f
30927cb5306d01da2716786c2d9457c49ec49d0f refactor: replace RecursiveMutex `m_subver_mutex` with Mutex (Sebastian Falbesoner)
0639aba42ad8449fe43eb2aad0ffbe3390aabd37 scripted-diff: rename `cs_SubVer` -> `m_subver_mutex` (Sebastian Falbesoner)
Pull request description:
This PR is related to #19303 and gets rid of the RecursiveMutex `cs_SubVer`. Both of the critical sections only directly access the guarded variable, i.e. it is not possible that within one section another one is called, and we can use a regular Mutex.
ACKs for top commit:
hebasto:
ACK 30927cb5306d01da2716786c2d9457c49ec49d0f, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 2faead792ea0b2f79f9f7fe99acde5cf2bfcd2f15c51fbb6cb1099d4f81276001a492f7d46a5139055f4366c2d58a36a8ba19f21d56df20e0ed93af3141dbe11
fa99e108e778b5169b3de2ce557af68f1fe0ac0b Fix implicit-integer-sign-change in arith_uint256 (MarcoFalke)
Pull request description:
This refactor doesn't change behaviour, but clarifies that the numbers being dealt with aren't supposed to be negative. This helps when reading the code and allows to remove a sanitizer suppression for the whole file.
ACKs for top commit:
PastaPastaPasta:
utACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
shaavan:
ACK fa99e108e778b5169b3de2ce557af68f1fe0ac0b
Tree-SHA512: f227e2fd22021e39f0445ec041f4a299d13477c23cef0fc06c53fb3313cbe550cec329336224a7e8775d9045b8009423052b394e83d42a1e40772085dfcdd471
fac22fd36b2d9f55dada31cc0da55520431b972a log: Remove GetAdjustedTime from IBD header progress estimation (MarcoFalke)
Pull request description:
This is a "refactor" that shouldn't change behaviour, because the two times are most likely equal. A minimum of 5 outbound peers are needed to adjust the time. And if the time is adjusted, it will be by at most 70 minutes (`DEFAULT_MAX_TIME_ADJUSTMENT`). Thus, the progress estimate should differ by at most 7 blocks.
ACKs for top commit:
laanwj:
Code review ACK fac22fd36b2d9f55dada31cc0da55520431b972a
vincenzopalazzo:
ACK fac22fd36b
Tree-SHA512: bf9f5eef66db0110dd268cf6dbfab64b9c11ba776924f5b386ceae3f2d005272cceb87ebcc96e0c8b854c051514854a2a5af39ae43bad008fac685b5aafaabd0
fa7238300c18938cdf627cacfc58d4b81602417f fuzz: Limit fuzzed time to years 2000-2100 (MarcoFalke)
Pull request description:
It doesn't make sense to fuzz times in the past, as Bitcoin Core will refuse to start in the past.
Fix that and also remove a sanitizer suppression, which would be hit in net_processing in `ProcessMessage`:
```cpp
if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
addr.nTime = nNow - 5 * 24 * 60 * 60; // <-- Here
```
This changes the format of fuzz inputs. Previously a time value was (de)serialized as 40 bytes, now it is 32 bytes.
ACKs for top commit:
mzumsande:
Code Review ACK fa7238300c18938cdf627cacfc58d4b81602417f
Tree-SHA512: ca6e7233beec2d9ef9fd481d8f1331942a4d2c8fe518b857629bebcc53a4f42ae123b994cf5d359384a0a8022098ff5a9c146600bc2593c6d88734e25bc240ad
In each of the critical sections, only the the guarded variable is
accessed, without any chance that within one section another one is
called. Hence, we can use an ordinary Mutex instead of RecursiveMutex.
-BEGIN VERIFY SCRIPT-
sed -i 's/std::string m_command;/std::string m_type;/g' ./src/net.h
sed -i 's/* command and size./* type and size./g' ./src/net.h
sed -i 's/msg.m_command/msg.m_type/g' ./src/net.cpp ./src/net_processing.cpp ./src/test/fuzz/p2p_transport_serialization.cpp
-END VERIFY SCRIPT-
Refactors TopUp so that it also tops up inactive chains. The bulk of
TopUp is moved to TopUpChain.
CHDChain also has 2 new in memory variables to track its highest used
indexes. This is used only for inactive hd chains so that they can be
topped up later in the same session (e.g. if the wallet is encrypted and
not unlocked at the time of MarkUnusedAddresses).
On a period boundary, getdeploymentinfo (and previously getblockchaininfo)
would report the status and statistics for the next block rather than
the current block. Change this to always report the status/statistics
of the current block, but add status-next to report the status for the
next block.
In each of the critical sections, only the the guarded variables are
accessed, without any chance that within one section another one is
called. Hence, we can use an ordinary Mutex instead of RecursiveMutex.
aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d test: use MiniWallet for mempool_accept.py (Sebastian Falbesoner)
b24f6c6855bdd09bf445faeebe9d54c3d07a46d9 test: MiniWallet: support default `from_node` for creating txs (Sebastian Falbesoner)
f30041c9143d0added18105c9f0c4ae3f340efbc test: create txs with current `nVersion` (2) by default (Sebastian Falbesoner)
2f7978682245ada40e7f72c6f28bf2379fdc8ce2 test: refactor: add constant for sequence number `SEQUENCE_FINAL` (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (mempool_accept.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in #20078.
It also includes some other minor changes that came up while working on the replacement:
* [commit 1/4] replace magic number 0xffffffff for a tx's nSequence with a new constant `SEQUENCE_FINAL`
* [commit 2/4] create `CTransaction` instances with the current nVersion=2 by default, in order to use BIP68 for tests
* [commit 3/4] support default `from_node` parameter for creating txs (this is a stripped down version of PR #24025)
ACKs for top commit:
MarcoFalke:
re-ACK aa8a65e4a88bfbd83ac756a87bfb8faf52fb675d 📊
Tree-SHA512: 34cd085ea4147ad5bd3f3321c84204064ceb95f382664c7fe29062c1bbc79d9d9465c5e46d35e11c416f2f3cd46030c90a09b518c829c73ae40d060be5e4c9cb
c62d763fc313585d79ad833c9d729f6acf2652aa Necessary improvements to make configure work without libevent installed (Perlover)
091ccc38c2e589b649648cbcc99aca4802f98775 The evhttp_connection_get_peer function from libevent changes the type of the second parameter. Fixing the problem. (Perlover)
Pull request description:
The second parameter of evhttp_connection_get_peer in libevent already has type as `const char **`
The compilation of bitcoind with the fresh libevent occurs errors
Details: https://github.com/bitcoin/bitcoin/issues/23606
ACKs for top commit:
laanwj:
Code review ACK c62d763fc313585d79ad833c9d729f6acf2652aa
luke-jr:
tACK c62d763fc313585d79ad833c9d729f6acf2652aa
Tree-SHA512: d1c8062d90bd0d55c582dae2c3a7e5ee1b6c7ca872bf4aa7fe6f45a52ac4a8f59464215759d961f8efde0efbeeade31b08daf9387d7d50d7622baa1c06992d83
