💬 0xB10C commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3237765557)
> Would such blocks mined by anyone even be considered valid on signet since they would not contain any expected valid signature?

Yes. Just the nonce in the block header changes. The signet commitment commits to the header version, prevhash, merkleroot, and timestamp. It does not commit (and can not commit) to the nonce. The underlying block does not change. The commitment remains valid.

💬 achow101 commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3237941668)
> Is this context info useful? Line numbers will change whenever some previous function in the file gets a non-trivial edit... Would it be better to add `-locations none` to the LCONVERT invocation in translate.cmake?

I don't think line numbers is the problem. The actual source location of the string can change and the ids in the `.xlf` won't change. It's the sort order in the `.ts` file that `lupdate` generates that seems to cause the id change. It's unclear to me why the sort order would ch
...

👍 l0rinc approved a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3169923720)
ACK fa3f682032a3292604f363a5ee4557937f3d8950

💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2311028761)
Done

💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048525)
Updated the commit message.

💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2311048812)
Clarified the test name

👍 l0rinc approved a pull request: "build: add `-Wleading-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482#pullrequestreview-3169950067)
Core review ACK 8c60dc39a353c9d66cbe3a5a0a934eca6b2a287a

💬 l0rinc commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311047700)
nit: there are a few remaining trailing spaces e.g. in https://github.com/bitcoin/bitcoin/blob/07f0a61ef711a2f75ded3d73545bfabdf2a64fef/src/minisketch/src/fields/clmul_common_impl.h#L39, we could add a PR to https://github.com/bitcoin-core/minisketch to get rid of those as well

💬 l0rinc commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2311037992)
nit: given that we're fixing non-trailing tabs as well, the only remaining .cpp/.h file that still contains tabs is https://github.com/bitcoin/bitcoin/blob/75a5c8258ec5309fe506438aa3815608430b53d6/src/util/subprocess.h#L1149, if you edit again, consider including that

💬 l0rinc commented on pull request "clang-format: align brace-after-struct and *-class formatting":
(https://github.com/bitcoin/bitcoin/pull/32813#discussion_r2311103606)
> Admittedly they are imprecise

I'd say ~85000 vs ~400 is a bit more than imprecise :)

> almost double on same line

That's not how we usually decide outcomes, rather by better explanations - especially since many of those aren't counting declarations, e.g
* https://github.com/bitcoin/bitcoin/blob/f7d2db28e90297664390d527881ebbbf2c6ce6f0/src/bitcoin-cli.cpp#L241
* https://github.com/bitcoin/bitcoin/blob/face8123fdc10549676c6679ee3225c178a7f30c/src/httpserver.cpp#L565
* https://github.
...

👋 l0rinc's pull request is ready for review: "stabilize translations by reverting old ids by text content"
(https://github.com/bitcoin/bitcoin/pull/33270)

🤔 enirox001 reviewed a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3170325100)
ACK fa3f682 - docstring cleanup, looks good.

💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3238341125)
> Are there any stats on increased coverage, or is this just checking more values in the same paths?

I'm going to share a coverage report but yes, it increases the coverage and reaches 100% of `wallet/fees.cpp`. One of the motivations for this PR is that I noticed that the following code was uncovered due to mempool min fee being always zero:

```cpp
// Obey mempool min fee when using smart fee estimation
CFeeRate min_mempool_feerate = wallet.chain().mempoolMinFee();
if (feerate_needed <
...

👍 pablomartin4btc approved a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3170406861)
ACK fa3f682032a3292604f363a5ee4557937f3d8950

(_Or adding back the assert correcting the value (0.00000100) and the comment (0.1)?_)

💬 l0rinc commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3238407406)
@maflcko, this is for every translation update, after this change previously translated values are kept and recognized by Transifex.

@achow101 created a test Bitcoin Core translation sandbox for me where I've uploaded the latest `bitcoin_en.xlf`.
Between Core versions most of the translations need to be reassigned (in the example of 29 vs 30 only 10% of the original IDs were kept so the translations all showed that most of the strings don't have pairs)
<img src="https://github.com/user-atta
...

💬 l0rinc commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3238412102)
I have checked the problem against the actual Core translations on Transifex and I could reproduce the massive invalidations and https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3238407406 does fix it successfully so that translators only need to check the modified entries:
<img src="https://github.com/user-attachments/assets/a90798da-fab9-4732-bc4a-075711e10558" />

🤔 w0xlt reviewed a pull request: "test: Use extra_port() helper in feature_bind_extra.py"
(https://github.com/bitcoin/bitcoin/pull/33260#pullrequestreview-3170731462)
ACK https://github.com/bitcoin/bitcoin/pull/33260/commits

Adding the patch below can confirm that the condition described in https://github.com/bitcoin/bitcoin/issues/33250#issuecomment-3225111710 is reached.

```diff
diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py
index 7dfa9c06ee..48b31092d2 100755
--- a/test/functional/feature_bind_extra.py
+++ b/test/functional/feature_bind_extra.py
@@ -27,7 +27,7 @@ class BindExtraTest(BitcoinTestFramewor
...

💬 w0xlt commented on pull request "test: Use extra_port() helper in feature_bind_extra.py":
(https://github.com/bitcoin/bitcoin/pull/33260#discussion_r2311675883)
If you apply the patch below, the patch above will work.
Is there a reason why the starting port index should be the same as the number of nodes?

```suggestion
extra_port.index = 0
```

💬 l0rinc commented on pull request "test: Use extra_port() helper in feature_bind_extra.py":
(https://github.com/bitcoin/bitcoin/pull/33260#discussion_r2311676593)
We're introducing extra confusion here now with assigning different types to the same variable.
I'm not suggesting a "rewrite", just to make it better than it was before - which it mostly is, hence the concept ack from me.

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2311525928)
what's the reason for keeping the old indentation here?