30cc1c6609 refactor: Drop `owns_lock()` call (Hennadii Stepanov)
bff4e068b6 refactor: Do not discard `try_lock()` return value (Hennadii Stepanov)
Pull request description:
Microsoft's C++ Standard Library uses the `[[nodiscard]]` attribute for `try_lock()`.
See: https://github.com/microsoft/STL/blob/main/stl/inc/mutex
This change allows to drop the current suppression for the warning C4838 and helps to prevent the upcoming warning C4858.
See: 539c26c923Fixesbitcoin/bitcoin#26017.
Split from bitcoin/bitcoin#25819.
ACKs for top commit:
vasild:
ACK 30cc1c6609
Tree-SHA512: ce17404e1c78af4f763129753caf8e5a0e1c91ba398778fe912f9fcc56a847e8112460d1a1a35bf905a593b7d8e0b16c6b099ad74976b67dca5f4f3eda6ff621
9cbfe40d8a net: remove useless call to IsReachable() from CConnman::Bind() (Vasil Dimov)
Pull request description:
`CConnman::Bind()` is called without `BF_EXPLICIT` only when passed
either `0.0.0.0` or `::`. For those addresses `IsReachable()` is always
true (regardless of the `-onlynet=` setting!), meaning that the `if`
condition never evaluates to true.
`IsReachable()` is always true for the "any" IPv4 and IPv6 addresses
because `CNetAddr::GetNetwork()` returns `NET_UNROUTABLE` instead of
`NET_IPV4` or `NET_IPV6` and the network `NET_UNROUTABLE` is always
considered reachable.
It follows that `BF_EXPLICIT` is unnecessary, remove it too.
ACKs for top commit:
naumenkogs:
ACK 9cbfe40d8a
aureleoules:
ACK 9cbfe40d8a
mzumsande:
ACK 9cbfe40d8a
Tree-SHA512: 4e53ee8a73ddd133fd4ff25635135b65e5c19d1fc56fe5c30337406560664616c0adff414dca47602948919f34c81073aae6bfc2871509f3912663d86750928e
91bee4d898 ci: Run `bench_bitcoin.exe --sanity-check` in "Win64 native" task (Hennadii Stepanov)
Pull request description:
This PR adds [`--sanity-check`](https://github.com/bitcoin/bitcoin/pull/25107) flag to `src\bench_bitcoin.exe` invocation as its results are been discarded.
Also a better name used for the script as it follows GNU's `make check`.
ACKs for top commit:
fanquake:
ACK 91bee4d898
Tree-SHA512: fd5feeda72d1ef46c5fbfc2aa5c042ab2e3de7772546379da4596306b5658ab95f62939fba237c0bd7a1b09c85de20fc1cd9e5df1efe11bdae50d4a7b8081f74
079cf88c0d refactor: move Boost datetime usage to wallet (fanquake)
Pull request description:
This means we don't need Boost Datetime in a `--disable-wallet` build, and it isn't included in the kernel (via time.h/cpp). Split from a larger boost removal branch/effort.
ACKs for top commit:
hebasto:
re-ACK 079cf88c0d
aureleoules:
re-ACK 079cf88c0d - rebased and two additional unit tests since my last review.
jarolrod:
crACK 079cf88c0d
Tree-SHA512: c84f47158a4f21902f211c059d8c4bd55ffe95a256835deee723653be08cca49eeddfc33a2316b0cd31805e81cf77eaa39c6c9dcff4cda11a26ba4c1c143974e
5c9a27a46f test: Use proper Boost macros instead of assertions (Hennadii Stepanov)
Pull request description:
On the master branch:
```
$ src/test/test_bitcoin -l test_suite -t banman_tests
Running 1 test case...
...
Test case banman_tests/file did not check any assertions
...
```
This PR suggests to use proper Boost [macros](https://www.boost.org/doc/libs/1_80_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref.html).
Top commit has no ACKs.
Tree-SHA512: e0c8e5e6371acd0e0a80070fffdf1445f264c62499f8d9811822994c89735a913c18c8ed730495578400abdd93d2d500345504f2a9246401d53fb2f9f71be8c5
51a08f41ff signet/miner: reduce default interblock interval limit to 30min (Anthony Towns)
Pull request description:
Reduces the cap on the time between blocks from 60 minutes to 30 minutes, and makes it configurable.
Top commit has no ACKs.
Tree-SHA512: 7b880c50e47d055a2737c057fab190017748849d264c6c39dde465959a544d502221d12c6307d4de693f51badb4779b9c147e48775ede6ec6613e808067ab279
a9d20eeceb doc: bump bips.md up-to-date version to v24.0 (Sebastian Falbesoner)
Pull request description:
This is a trivial follow-up to #26124.
ACKs for top commit:
jarolrod:
ACK a9d20eeceb
Tree-SHA512: 24c17c72498f96f9122d8fb041f1f6f63bd186e25ac3cb5a661bb1993106c6632f5efd95a15d19681004d30d38eca2d2a16b383a7a1f1c3db17f887ae1fcd02a
0811cbfc28 doc: add info about status code 404 for some rest endpoints (brunoerg)
Pull request description:
This PR adds an explanation about status code 404 for 2 endpoints (`/rest/tx/ `and `/rest/blockhashbyheight/`) in`REST-interface.md`. There are other endpoints that already cover it.
ACKs for top commit:
[deleted]:
reACK 0811cbfc28
shaavan:
ACK 0811cbfc28
Tree-SHA512: a01ac6653f706b7a7e4a4679a2b81e448381f31460ac4bcfc179af6186401cffae7b49a82f3a52c89e556acd5c16c159ce752c7a678177900ddf2e4e5c72fe6b
be59bd17ec contrib: Fix capture_output in getcoins.py (willcl-ark)
Pull request description:
Our minimum required Python version 3.6.12 does not support `capture_output` as a subprocess.run argument; this was added in python 3.7.
We can emulate it by setting both stdout and stderr to `subprocess.PIPE`
Top commit has no ACKs.
Tree-SHA512: 3158bfe657e2d5b263c9337a7ac1dd830e7e119854ec54a40a9e511329fe5bed073350bf70c66b5a6cc9a27dc8eb0e062bbac3969640fd7d6475379726003c98
9d14f27bdd log: log RPC port on startup (James O'Beirne)
Pull request description:
I just spent a few hours trying to figure out why "18444" wasn't getting me to regtest's RPC server. I'm not the sharpest tool in the shed, but I was maybe understandably confused because "Bound to 127.0.0.1:18445" appears in the logs, which I assumed was the P2P port.
This change logs the RPC listening address by default on startup, which seems like a basic piece of information that shouldn't be buried under `-debug`.
ACKs for top commit:
dergoegge:
ACK 9d14f27bdd
jarolrod:
ACK 9d14f27bdd
aureleoules:
ACK 9d14f27bdd
Tree-SHA512: 5c86f018c0b8d6264abf878c921afe53033b23ab4cf289276bb1ed28fdf591c9d8871a4baa4098c363cb2aa9a637d2e4e18e56b14dfc7d767ee40757d7ff2e7c
Since m_next_resend is now only called from MaybeResendWalletTxs()
we don't have any potential race conditions anymore, so the usage
of std::atomic can be reverted.
We only want to relay our resubmitted transactions once every 12-36h.
By separating the timer update logic out of ResubmitWalletTransactions
and into MaybeResendWalletTxs we avoid non-relay calls (previously in
the separate ReacceptWalletTransactions function) from resetting that
timer.
Moves the logic of whether or not transactions should actually be
resent out of the function that's resending them. This reduces
responsibilities of ResubmitWalletTransactions and allows
carving out the updating of m_next_resend in a future commit.
Our required Python version 3.6.12 does not support `capture_output` as
a subprocess.run argument; this was added in python 3.7.
We can emulate it by setting stdout and stderr to subprocess.PIPE
fa2c72dda0 rpc: Set RPCArg options with designated initializers (MacroFake)
Pull request description:
For optional constructor arguments, use a new struct. This comes with two benefits:
* Earlier unused optional arguments can be omitted
* Designated initializers can be used
ACKs for top commit:
stickies-v:
re-ACK fa2c72dda0
Tree-SHA512: 2a0619548187cc7437fee2466ac4780746490622f202659f53641be01bc2a1fea4416d1a77f3e963bf7c4cce62899b61fab0b9683440cf82f68be44f63826658
fad7281d78 ci: Use remote pull/merge ref instead of local git merge (MacroFake)
Pull request description:
The merge strategy on the remote may be different than the local one. This may cause local merges to be different or fail completely. Fix this by using the result of the remote merge.
Fixes https://github.com/bitcoin/bitcoin/issues/26163
ACKs for top commit:
hebasto:
ACK fad7281d78, I regularly use the same commands locally.
Tree-SHA512: 0febbf5db8c1536e31b374a7599a92037ca814174809075f42c7c7c4e1daaab5b3df09cf82f2de0d1e847c41eb30e770daaf7a85287f5d8d43ebd642d1234d3c
The merge strategy on the remote may be different than the local one.
This may cause local merges to be different or fail completely. Fix this
by using the result of the remote merge.
faeea28753 test: Avoid race in disconnect_nodes helper (MacroFake)
Pull request description:
Also wait for the other node to notice the closed socket. Otherwise, the other node is not able to use the connect helper.
Fixes https://github.com/bitcoin/bitcoin/issues/26014
ACKs for top commit:
stickies-v:
ACK faeea2875
glozow:
ACK faeea28753
Tree-SHA512: 2f0fa6812c0519aba3eaf21f0c70073b768fcd4dad23989d57e138ee9057a7da1a6b281645e9bff4051259cdca51568700e066491ac6b6daae99f30e395159ca
d8ded8bc08 ci: Use git2.34 for lint task (MacroFake)
Pull request description:
Since most maintainers use a recent version of git that uses the `ort` strategy by default (https://git-scm.com/docs/merge-strategies/2.34.0), bump git for the lint taks as well.
Fixes https://github.com/bitcoin/bitcoin/pull/26130#issuecomment-1260499544
ACKs for top commit:
fanquake:
ACK d8ded8bc08 - seems fine for now, and to keep python3.6 around. When we bump to >= Jammy in future we'll have to pick from Python3.10+.
Tree-SHA512: 5a9c40b1c242678a7f92e641db026309b3e2e99d7d032778c98eeb56f7abd65f9e0a24f9b2ccf0350d5c0286d50f1ac5969e4249beaa5ffc4b00d06ca8b141bc
Microsoft's C++ Standard Library uses the `[[nodiscard]]` attribute for
`try_lock()`.
See: https://github.com/microsoft/STL/blob/main/stl/inc/mutex
This change allows to drop the current suppression for the warning C4838
and helps to prevent the upcoming warning C4858.
See: 539c26c923
bdcafb9133 p2p: ProcessHeadersMessage(): fix received_new_header (Larry Ruane)
Pull request description:
Follow-up to #25717. The commit "Utilize anti-DoS headers download strategy" changed how this bool variable is computed, so that its value is now the opposite of what it should be.
Prior to #25717:
```
bool received_new_header{WITH_LOCK(::cs_main, return m_chainman.m_blockman.LookupBlockIndex(headers.back().GetHash()) == nullptr)};
```
After #25717 (simplified):
```
{
LOCK(cs_main);
last_received_header = m_chainman.m_blockman.LookupBlockIndex(headers.back().GetHash());
}
bool received_new_header{last_received_header != nullptr};
```
ACKs for top commit:
dergoegge:
ACK bdcafb9133
glozow:
ACK bdcafb9133, I believe this is correct and don't see anything to suggest the switch was intentional.
stickies-v:
ACK bdcafb9133
Tree-SHA512: 35c12762f1429585a0b1c15053e310e83efb28c3d8cbf4092fad9fe81c893f6d766df1f2b20624882acb9654d0539a0c871f587d7090dc2a198115adf59db3ec
810c3dc7ef doc, rpc: mention that `listdescriptors` result is sorted by string representation (Sebastian Falbesoner)
d99af861d0 test: check that `listdescriptors` descriptor strings are sorted (Sebastian Falbesoner)
Pull request description:
This small PR adds a test for the change introduced in PR #25931 ("rpc: sort listdescriptors result", commit 50996241f2). The correctness of the test can easily be verified by commenting out the `std::sort` call in the `listdescriptors` RPC implementation:
```diff
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 09c74ea2da..3ed1a69b26 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -1829,9 +1829,11 @@ RPCHelpMan listdescriptors()
});
}
+ /*
std::sort(wallet_descriptors.begin(), wallet_descriptors.end(), [](const auto& a, const auto& b) {
return a.descriptor < b.descriptor;
});
+ */
UniValue descriptors(UniValue::VARR);
for (const WalletDescInfo& info : wallet_descriptors) {
```
leading to a fail of the functional test `wallet_listdescriptors.py`.
ACKs for top commit:
jarolrod:
ACK 810c3dc7ef
aureleoules:
ACK 810c3dc7ef
Tree-SHA512: 31770e3149b8a0251ecfa8662a2270c149f778eb910985f48a91d6a5d288b7b1c2244f9f1b798ebe3f1aa9f0b935cb4d6f12d5d28f78bcde3c4a61af76d11d0a
