Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 l0rinc commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2173852408)
Opened it separately in https://github.com/bitcoin/bitcoin/pull/32827, please resolve this comment
📝 l0rinc converted_to_draft a pull request: "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827)
During Initial Block Download, the mempool is known to be empty, but `CTxMemPool::removeForBlock` is still called for every connected block where we:
* iterate over every transaction in the block even though none will be found in the empty `mapTx`, always leaving `txs_removed_for_block` empty...
* which is pre-allocated regardless with 40 bytes * vtx.size(), even though it will always remain empty.

Similarly to https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140691354, this chang
...
💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2173853678)
I think `COMPONENT Kernel` should become `COMPONENT libbitcoinkernel`. Noticed when running `cmake --install build --component libbitcoinkernel` the header file wasn't being installed.
👍 l0rinc approved a pull request: "p2p: add more bad ports"
(https://github.com/bitcoin/bitcoin/pull/32826#pullrequestreview-2969371289)
ACK 23a7807ec6d35d3ef5b65e4abc3b3841e84b702e

Verified that these are indeed the default ports and that the docs match the code.
Have a miner test suggestion, LGTM otherwise.
💬 l0rinc commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173855831)
instead of updating the comment to match the code (which is the ultimate source of truth anyway), seems like a good opportunity to remove the duplication of the code below - which is already trivial, no need for a dead comment that doesn't add any new info.
💬 l0rinc commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173861432)
I was wondering if we could do a more functional approach here, since we already have a few `count_if` instances - not sure it's better or not, will let you decide, please resolve if you don't think it's a good alternative (assuming integer promotion for max + 1):
```suggestion
using namespace std::ranges;
auto ports{views::iota(1, std::numeric_limits<uint16_t>::max() + 1)};
BOOST_CHECK_EQUAL(count_if(ports, IsBadPort), 85);
```
📝 hebasto opened a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/32828)
The `boost-headers` package does not provide CMake configuration files required since https://github.com/bitcoin/bitcoin/pull/32667. Install the `boost` package instead.
📝 hebasto converted_to_draft a pull request: "doc: Update NetBSD Build Guide"
(https://github.com/bitcoin/bitcoin/pull/32828)
The `boost-headers` package does not provide CMake configuration files required since https://github.com/bitcoin/bitcoin/pull/32667. Install the `boost` package instead.
💬 jlopp commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173890219)
implemented
📝 kevkevinpal opened a pull request: "threading: use correct mutex name in reverse_lock fatal error messages"
(https://github.com/bitcoin/bitcoin/pull/32829)
"Now that REVERSE_LOCK requires the name of the actual mutex, it can be used for better error messages." - theuni

This is a follow-up to this comment https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2981287545

I just cherry-picked the commit 85c2848eb575f4abaa81fdd4e8f3b2048693dd98
🤔 l0rinc reviewed a pull request: "p2p: add more bad ports"
(https://github.com/bitcoin/bitcoin/pull/32826#pullrequestreview-2969413138)
ACK 8716fd2f4a00b07426e54a1491af421d6345f708
💬 l0rinc commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173890816)
might as well inline it now that it's only used once
```suggestion
BOOST_CHECK_EQUAL(std::ranges::count_if(ports, IsBadPort), 85);
```
💬 l0rinc commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173892111)
👍, looks like this contains the end as well, no +1 needed.
Checked with `BOOST_CHECK_EQUAL(std::ranges::count_if(ports, [&](const int i) { return i == std::numeric_limits<uint16_t>::max(); }), 1);` to make sure.

---

Note though that this is a substantial memory allocation, the previous suggestion was lazy.
For the record here's the complete lazy solution I suggested (with fixed imports):
```patch
diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp
index e3bf482873..
...
💬 kevkevinpal commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#issuecomment-3017035899)
ACK [8716fd2](https://github.com/bitcoin/bitcoin/pull/32826/commits/8716fd2f4a00b07426e54a1491af421d6345f708)

It makes sense to me to add these ports since they're popular and used. I double-checked the links @l0rinc provided and also did a quick search myself to make sure they were correct
💬 jlopp commented on pull request "p2p: add more bad ports":
(https://github.com/bitcoin/bitcoin/pull/32826#discussion_r2173897398)
done