🚀 achow101 merged a pull request: "assumeutxo: Don't load a snapshot if it's not in the best header chain"
(https://github.com/bitcoin/bitcoin/pull/30320)

💬 achow101 commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#issuecomment-2237670866)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb

👍 hebasto approved a pull request: "depends: build FreeType with CMake"
(https://github.com/bitcoin/bitcoin/pull/29880#pullrequestreview-2186962592)
ACK ff4f3deb7b8adfcc90fb745440ce4be1176552ca, I've verified the actual compile options, they look sane.

🚀 achow101 merged a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245)

💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#issuecomment-2237789775)
Rebased and since this is the last of the PRs addressing the TODOs comment in `feature_assumeutxo.py`, it now removes that whole section.

📝 theStack opened a pull request: "test: add creating/spending validity checks for rare output scripts"
(https://github.com/bitcoin/bitcoin/pull/30481)
This PR adds a functional test with a collection of output scripts that are "rare"/obscure, in the sense that they wouldn't be used or make sense within a regular payment transaction, but creating them is still considered standard, which is often considered surprising and counter-intuitive. They either
* use a weird pubkey encoding (-> hybrid pubkeys [in types P2PK, P2MS]),
* are provably unspendable (-> not-on-curve pubkeys [in types P2PK, P2MS, P2TR]), or
* are spendable by anyone with curr
...

💬 petertodd commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2237833642)
> @petertodd I'll give a non-TRUC version another try and report back. I may have been over-thinking things. If it works out it will be very similar in code structure...
>
> edit: oh of course I already noted in OP: We don't allow individual transactions with 0-fee outside of TRUC, for now due to trimming computation concerns. Cluster mempool does away with this restriction entirely. I'll rework the code to upgrade to non-TRUC usage once cluster mempool hits.

I think the worthwhile questio
...

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683669595)
The other things can be safely set to false, so we can be sure they're false without an assertion. But if you've set a callback you've also gotten an `iterator` that you might intend to pass to `DeleteCallback` later, which would then crash if we had cleared `m_print_callbacks` here.

🤔 ryanofsky reviewed a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2187172873)
re: https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2185910679

> > but internally it is calling the uint256S function which expects a nul-terminated string
>
> Maybe I'm being too pedantic (or wrong), but I'm not sure this is 100% correct? I don't think the `uint256S(std::string_view str)` `str` needs to be null-terminated.

Yes, both explanations are saying the same thing but mine is unnecessarily confusing. The `uint256S` function "expects" a nul-terminated string becau
...

💬 ryanofsky commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683689226)
re: https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1682892102

> review note: it seems the `while` loop was replaced with `for` loop purely for readability, I don't see any functional change here.

To be sure, replacing the previous while loop is an important part of the fix. If the previous while loop was kept, it could iterate past the end of the of `string_view` and read uninitialized memory. This for loop (and the while loop I suggested in https://github.com/bitcoin/bitcoin/pu
...

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683742290)
Added a `[...]` marker, since it was easy.

In order to trigger it you'd need to have 1MB of log message on a single, incomplete line, which seems a bit ridiculous. Long-term, I think it would make more sense to stop intentionally doing incomplete lines (since that introduces a race condition if other threads are also logging), and just add a newline if it's missing... *shrug*

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683742446)
Changed to strprintf. Got a bit target fixated there...

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683742656)
That's a bug, changed.

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683742718)
Done

💬 ajtowns commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683745201)
I'd like to change `Enabled()` and `WillLogCategoryLevel()` functions to be lockless so that calls to `Log*()` don't block when they have nothing to do. I think the above changes would mesh well with that.

💬 maflcko commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683805668)
Fair enough, but I am mostly thinking what would be the use case for possibly confusing code like `m_print_to_console=true;DisableLogging()`? Given that this function also `assert(m_buffering)`, the code can only be called when logging was never active, I'd still say it makes sense to at least `Assume(!m_print_to*)`.

💬 maflcko commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683814101)
nit in 1cd8386a18db0ced6da0c5f42e952e81fa02867c:

```
clang-tidy-18 -p=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/logging.cpp
logging.cpp:19:13: error: using decl 'ToString' is unused [misc-unused-using-decls,-warnings-as-errors]
19 | using util::ToString;
| ^
logging.cpp:19:13: note: remove the using
19 | using util::ToString;

...

💬 maflcko commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#discussion_r1683814416)
nit in https://github.com/bitcoin/bitcoin/commit/1cd8386a18db0ced6da0c5f42e952e81fa02867c:

Could use `RemovePrefixView` for consistency and to avoid a useless string constructor?

💬 maflcko commented on pull request "Early logging improvements":
(https://github.com/bitcoin/bitcoin/pull/30386#issuecomment-2238236358)
lgtm, after a CI fix.

re-ACK 1cd8386a18db0ced6da0c5f42e952e81fa02867c 🏡

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: r
...

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2238241424)
> > are lacking a good description that actually says what the bug is.
>
> I agree, I spent quite a bit of time getting to the bottom of this and more documentation would have sped that up

I agree as well. Thanks for brining it up! This should be fixed before merge, other than that, I'd say it is ready.