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
We currently perform the same check twice, to put the same set of flags
in two different variables. Split the checks so we test for crc and crypto
extensions independently.
If we don't want to split, we should just delete the second AX_CHECK_COMPILE_FLAG
check, and set ARM_CRC_CXXFLAGS & ARM_CRC_CXXFLAGS at the same time.
73ae72e603 build: remove unused MSVC defines (fanquake)
Pull request description:
Most of these aren't unused in the codebase at all, let alone for MSVC.
ACKs for top commit:
sipsorcery:
tACK 73ae72e603.
Tree-SHA512: 6a8aef69902d5941fba741777c19176b96d11a4c75cf27bdf3881019fc650e8f60dd7960a1bc9469a24c4575f7d34cf675afdcf4bcb240c32cab1c9c6ab286db
553ff452c0 build: remove stdlib.h from header checks (fanquake)
a63d4cb26a refactor: use <cstdlib> over stdlib.h (fanquake)
Pull request description:
We already use a mix of `<cstlib>` and `stdlib.h` unconditionally throughout
the codebase.
Us checking this header also duplicates work already done by autotools.
Currently stdlib.h is checked for 3 times during a ./configure run, after
this change, at least it's only twice.
Similar to #26150.
ACKs for top commit:
kristapsk:
ACK 553ff452c0
TheCharlatan:
ACK 553ff452c0
Tree-SHA512: 0a43d39d3df180a1614dbd3a1ee1531b0969ffe4a0c09dfe9d2f3f0ec16196b5fd7523309f6722936a8c8b20908508724e1903e939dd81c3b4538d85d0f42953
a60d9eb9e6 Bugfix: Wallet: Lock cs_wallet for SignMessage (Luke Dashjr)
Pull request description:
cs_desc_main is typically locked within scope of a cs_wallet lock, but:
CWallet::IsLocked locks cs_wallet
...called from DescriptorScriptPubKeyMan::GetKeys
...called from DescriptorScriptPubKeyMan::GetSigningProvider which locks cs_desc_main first, but has no access to cs_wallet ...called from DescriptorScriptPubKeyMan::SignMessage ...called from CWallet::SignMessage which can access and lock cs_wallet
Resolve the out of order locks by grabbing cs_wallet in CWallet::SignMessage first
-------------
Note this is currently only an issue for the GUI (which lacks sufficient testing apparently), but can be reproduced by #26082 (CI fails as a result)
ACKs for top commit:
achow101:
ACK a60d9eb9e6
w0xlt:
ACK a60d9eb9e6
Tree-SHA512: 60f6959b0ceaf4d9339ba1a47154734034b637c41b1f9e26748a2dbbc3a2a95fc3696019103c55ae70c91d910ba8f3d7f4e27d263030eb60b689f290c4d82ea9
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.
We already use a mix of <cstdlib> and stdlib.h unconditionally throughout
the codebase.
Us checking this header also duplicates work already done by autotools.
Currently stdlib.h is checked for 3 times during a ./configure run, after
this change, at least it's only twice.
to the current p2p behavior. We only initialize the Peer::TxRelay m_relay_txs
data structure if it isn't an outbound block-relay-only connection and fRelay=true
(the peer wishes to receive tx announcements) or we're offering NODE_BLOOM to this peer.
