Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ‘‹ glozow's pull request is ready for review: "package validation: relax the package-not-child-with-unconfirmed-parents rule"
(https://github.com/bitcoin/bitcoin/pull/31385)
πŸ‘ hodlinator approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2477648972)
ACK f4df783f1ca22d96476d52ec5d1929547691ba13

Increased validation of format strings. :+1:

---

Maybe amend PR summary or final commit by spelling out something like:

Delays `bilingual_str`-construction until the end of the overload of `tinyformat::format` in *translation.h*. The `fmt` parameter to the overload is changed from `bilingual_str` -> `util::bilingual_fmt`, the latter being a new type which contains a `ConstevalFormatString`. `bilingual_fmt` in turn is only constructible fr
...
πŸ’¬ hodlinator commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1869532227)
nit: Can't we just be explicit here in the test to document what is happening, since it is slightly tricky?
```suggestion
constexpr util::Translatable<false> format{Untranslated("original [%s]")};
```
πŸ’¬ hodlinator commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1868949895)
nit:
```suggestion
std::string translate() const { return translatable ? util::translate(original.fmt) : original.fmt; }
```
⚠️ fanquake opened an issue: "test: failure in `interface_usdt_net.py`"
(https://github.com/bitcoin/bitcoin/issues/31418)
Master @ ae69fc37e4fff237a119225061d68f69e6cd61d7.
Running on Rawhide.
GCC: `gcc (GCC) 14.2.1 20241104 (Red Hat 14.2.1-6)`.
Systemtap: `systemtap-5.2-1.fc42.src.rpm`

```bash
cmake -B build -DWITH_USDT=ON
cmake --build build -j17
./build/test/functional/test_runner.py --jobs=17
```

```bash
123/314 - interface_usdt_net.py failed, Duration: 3 s

stdout:
2024-12-04T14:13:14.071000Z TestFramework (INFO): PRNG seed is: 2457885293535030371
2024-12-04T14:13:14.072000Z TestFramework (IN
...
πŸ’¬ ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1869619567)
The version number is always going to be displayed whether you run `bitcoind` with `-version` `-help` or no arguments at all, because it is logged at startup. If you don't want to see the version number you need to run `bitcoind` with `-noprinttoconsole` or `-daemon` to not log to the console.

The point of `-version` is to show the version number and copyright information and exit, and the point of `-noversion` is to do the opposite and start up normally. You can think of `-version` as being
...
πŸ’¬ l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1869626885)
I'm not yet sure this makes the behavior clearer for the version parameter, but the rest seems fine to me, thanks for clarifying.
πŸ’¬ maflcko commented on issue "test: failure in `interface_usdt_net.py`":
(https://github.com/bitcoin/bitcoin/issues/31418#issuecomment-2517585497)
cc @0xB10C
πŸ’¬ fanquake commented on issue "test: failure in `interface_usdt_net.py`":
(https://github.com/bitcoin/bitcoin/issues/31418#issuecomment-2517588711)
This also happens on `28.x`.
πŸ’¬ l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2517591742)
utACK 95a0104f2e9869799db84add108ae8c57b56d360
The code seems better than before, though most of these negations seem confusing to me - but it wasn't introduced here.
I have tested the negation of the parameters and reviewed most of the code - but couldn't meaningfully opine on the cookie related new tests, I will leave that to other reviewers.
πŸ‘ hodlinator approved a pull request: "refactor: Prepare compile-time check of bilingual format strings"
(https://github.com/bitcoin/bitcoin/pull/31295#pullrequestreview-2478868111)
cr-ACK fa3e074304780549b1e7972217930e34fa55f59a

#### Before
```C++
strprintf(_("Copyright (C) %i-%i").translated, 2009, COPYRIGHT_YEAR)
```
1. Create a `bilingual_str` of `"Copyright (C) %i-%i"`.
1. Calls `G_TRANSLATION_FUN("Copyright (C) %i-%i")`, getting something like `"SzerzΕ‘i jog (c) %i-%i"`, and storing it in `.translated`.
2. Perform regular `strprintf()` (`tfm::format(std::string...)`) on the returned `.translated` and substitute in the values, yielding `"SzerzΕ‘i jog (c) 2009-2
...
πŸ’¬ maflcko commented on issue "test: failure in `interface_usdt_net.py`":
(https://github.com/bitcoin/bitcoin/issues/31418#issuecomment-2517604299)
I guess an easy fix would be to just rename `MIN` to `net_tracepoints_MIN`?
πŸ’¬ glozow commented on pull request "p2p: track and use all potential peers for orphan resolution":
(https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1869654727)
Ah, need to add the size of `unique_parents` to this. Surprised fuzzer has not tripped on it...
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2517621750)
Updated 02567bf2bef5997ea5f0765780d196f36d3053e8 -> 2def75a15deeb39fa66b1e8d45679ac29fd18b82 ([`pr/wrap.6`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.6) -> [`pr/wrap.7`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.7), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.6..pr/wrap.7)) to fix argument parsing bug causing a test failure on windows https://github.com/bitcoin/bitcoin/actions/runs/12147886951/job/33875123699?pr=31375#step:12:95
πŸ’¬ instagibbs commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-2517668696)
> Original motivation for this rule was based on the idea that we would have a child-with-unconfirmed-parents package relay protocol, and this would verify that the peer provided the "correct" package

Was the idea that the full ancestor package was to be "pulled" into the package validation logic, including in-mempool things?
πŸ“ 0xB10C opened a pull request: "test: fix MIN macro redefinition"
(https://github.com/bitcoin/bitcoin/pull/31419)
Renames the `MIN` macro to `_TRACEPOINT_TEST_MIN`.

From #31418:

```
stderr:
/virtual/main.c:70:9: warning: 'MIN' macro redefined [-Wmacro-redefined]
70 | #define MIN(a,b) ({ __typeof__ (a) _a = (a); __typeof__ (b) _b = (b); _a < _b ? _a : _b; })
| ^
include/linux/minmax.h:329:9: note: previous definition is here
329 | #define MIN(a,b) __cmp(min,a,b)
| ^
1 warning generated.
```

fixes: https://github.com/bitcoin/bitcoin/issues/31418
πŸ’¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1869695020)
Yes, that is the case:

> argsman.AddArg("-maxconnections=<n>", ... It does not apply to short-lived private broadcast connections either, which have a separate limit of MAX_PRIVATE_BROADCAST_CONNECTIONS

You can run with `-debug=privatebroadcast` and see what happens when you send a new transaction.
πŸ’¬ 0xB10C commented on issue "test: failure in `interface_usdt_net.py`":
(https://github.com/bitcoin/bitcoin/issues/31418#issuecomment-2517674972)
> I guess an easy fix would be to just rename `MIN` to `net_tracepoints_MIN`?

doing this in https://github.com/bitcoin/bitcoin/pull/31419
πŸ€” glozow reviewed a pull request: "BlockAssembler: return selected packages virtual size and fee"
(https://github.com/bitcoin/bitcoin/pull/30391#pullrequestreview-2478957546)
ACK 5171f16e6f96030fcc91b30bb196a2790e37848c
πŸ’¬ vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2517703912)
I think the `tcpdump` approach works well enough. I am posting here just for the records - an alternative approach is to use `iptables` to detect traffic. I managed to get it to log the traffic, but couldn't read that log - I could only see a summary of how many packets matched, not individual ones and their destination address.

Below is a WIP patch which can be used as a starting point if I or somebody else wants to pursue that further.

<details>
<summary>[patch] WIP iptables</summary>

...