Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👍 dergoegge approved a pull request: "test: migrate to some per-symbol ubsan suppressions"
(https://github.com/bitcoin/bitcoin/pull/28865#pullrequestreview-1730221446)
utACK fd30e9688e15fe6e0f8b64202a6e9c7d96333394 (if CI is green)

Good stuff
💬 ismaelsadeeq commented on pull request "wallet: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1392912274)
Thanks Fixed
📝 fanquake opened a pull request: "Redundant upnp ifdef"
(https://github.com/bitcoin/bitcoin/pull/28874)
This is a very belated followup to #26896 (which removed the configure options for setting the upnp and natpmp runtime default) and corrects the `-help` docs for `-upnp` and `-natpmp`.
💬 jonatack commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1392921326)
True. I'm touching that code for #28248, if you have a naming suggestion.
💬 achow101 commented on pull request "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs":
(https://github.com/bitcoin/bitcoin/pull/28868#issuecomment-1810687500)
> Is (some part of) this meant for backport to 26.x?

With the current test failure that is hard to debug, no.
📝 hebasto opened a pull request: "build: Pass sanitize flags to instrument `libsecp256k1` code"
(https://github.com/bitcoin/bitcoin/pull/28875)
This PR is a revived https://github.com/bitcoin/bitcoin/pull/27991 with an addressed [comment](https://github.com/bitcoin/bitcoin/pull/27991#discussion_r1252148488).

Fixes https://github.com/bitcoin/bitcoin/issues/27990.

Might be tested as follows:
```
$ ./autogen.sh && ./configure --enable-fuzz --with-sanitizers=fuzzer CC=clang-13 CXX=clang++-13
$ make clean > /dev/null && make
$ objdump --disassemble=secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep __sanitizer_cov
1953bd0
...
💬 achow101 commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1810695939)
> Is it possible to support descriptors that include only the "aggregated xpub" and not every key as in the proposed `agg()` expression, leading to a O(1) size instead of O(number-of-keys)?

The aggregated pubkey will look like any other pubkey, so it is trivial to substitute it in to the descriptor if you wish to do so. However this is lossy as you will lose the information about the pubkeys that are involved in that aggregated pubkey, and that information is necessary for spending. It will s
...
💬 jonatack commented on pull request "[26.x] rc3 or finalize":
(https://github.com/bitcoin/bitcoin/pull/28872#issuecomment-1810696940)
> there is currently nothing further marked for backport to 26.x

Would suggest backporting #28155 as mentioned in https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1785999512.

Am rebasing and updating #28248 today, which fixes a number of bugs including the regression reported in https://github.com/bitcoin/bitcoin/pull/27213/files#r1291926369 and described in https://github.com/bitcoin/bitcoin/pull/28248/commits and https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-178970
...
💬 dergoegge commented on pull request "fuzz: AutoFile with XOR":
(https://github.com/bitcoin/bitcoin/pull/28873#issuecomment-1810700861)
Concept ACK
💬 dergoegge commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#issuecomment-1810702008)
Concept ACK

Thanks! will review soon
💬 achow101 commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1810708407)
#26711 was closed. Could we get a longer explanation of why and what the plan is going forward? My understanding is that the plan is to do a limited package relay carveout for just CPFP, then cluster mempool, then the rest of package relay?
💬 achow101 commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1810710622)
> The docs don't mention `make check`

Hmm? It's right there in the first sentence of that section:

> LCOV can be used to generate a test coverage report based upon `make check` execution.
💬 jonatack commented on pull request "test: fix node index bug when comparing peerinfo":
(https://github.com/bitcoin/bitcoin/pull/28849#issuecomment-1810715496)
Good eye, @kashifs 👍
💬 dergoegge commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1810722197)
> Hmm? It's right there in the first sentence of that section:
>
>> LCOV can be used to generate a test coverage report based upon make check execution.

Isn't that just saying that the generated coverage report covers the code executed in `make check`, not that you have to invoke `make check` prior to `make cov`? (that's at least how i read it and how `make cov` currently works). Since there are multiple interpretations, it probably makes sense to clarify.
🤔 stickies-v reviewed a pull request: "test: refactor: use built-in collection types for type hints (Python 3.9 / PEP 585)"
(https://github.com/bitcoin/bitcoin/pull/28725#pullrequestreview-1730313930)
Approach ACK 4b9afb18e6b9e16d7b299820f3a1382986a451d4

Reducing boilerplate for type hints is nice, and even though there's no deprecation date set, from PEP585 it appears the intent is for the `typing` types is to be deprecated in favor of the generic types.

From PEP585, it's also recommended to replace `Callable` and `Iterable` with their `collections.abc` alternative, done is this diff:

<details>
<summary>git diff on 4b9afb18e6</summary>

```diff
diff --git a/contrib/seeds/asmap.p
...
🤔 theStack reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1730319269)
I've noticed that the test `p2p_v2_encrypted.py` fails if the node under test sends a small amount of garbage (in the range of 0-19 bytes). Can be reproduced by applying the following simple patch:
```diff
diff --git a/src/net.cpp b/src/net.cpp
index 2682035912..09af15a9b7 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -951,7 +951,7 @@ std::vector<uint8_t> GenerateRandomGarbage() noexcept
{
std::vector<uint8_t> ret;
FastRandomContext rng;
- ret.resize(rng.randrange(V2Trans
...
💬 instagibbs commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1810800093)
I'll update Ephemeral Anchors PR/BIP once current set of PRs are merged, since I build on https://github.com/bitcoin/bitcoin/pull/28848 for testing
📝 hebasto opened a pull request: "build: Fix LTO functionality"
(https://github.com/bitcoin/bitcoin/pull/28876)
The current LTO functionality implementation has multiple issues:
1. Duplicated `-flto` flags when linking.
2. The `secp256k1` subtree code is built without LTO.
3. In `AX_CHECK_LINK_FLAG` macro the wrong `CXXFLAG_WERROR` flag is used instead of the correct `LDFLAG_WERROR` one.

This PR fixes all of them.

Might be verified as follows:
```
$ readelf -S src/secp256k1/src/libsecp256k1_la-secp256k1.o | grep .gnu.lto_ | wc -l
274
```

---

The depends build system has similar issues t
...
💬 glozow commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1810836012)
There have been discussions on what package validation and RBF rules will look like in a post-cluster mempool world. #26711 either doesn't work with cluster mempool or should not be considered until after cluster mempool. #26711 supports certain types of package submission that we might not be able to continue to support with cluster mempool depending on what approach is taken. There are different ideas on how to consider subsets of packages to accept when it doesn't make sense to take all or no
...
💬 glozow commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1810839054)
The OP has been updated to reflect the new plan.