💬 l0rinc commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767009245)
Is the `LogFlags::NONE` -> `BCLog::NET` category in case of `manual_connection` a behavior change?

💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2361325488)
Rebased on master and adapted the `dumptxoutset` call in the test to the new interface (see #29553).

✅ mcelrath closed an issue: "Race condition between ZMQ UpdateTip and getblocktemplate"
(https://github.com/bitcoin/bitcoin/issues/30862)

💬 mcelrath commented on issue "Race condition between ZMQ UpdateTip and getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/30862#issuecomment-2361325732)
@mzumsande you are right. Apparently I failed to scroll up in the log. There was several tips announced simultaneously and this is absolutely due to a previous invocation of `getblocktemplate`. I'm going to close issue this now, as I've also failed to reproduce it since.

Log with more context attached.

[miner.log](https://github.com/user-attachments/files/17061890/miner.log)

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767048974)
Thanks for pointing out the behaviour change, I've dropped the commit.

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767049495)
Taken (minus string literals), thanks!

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767050387)
It is indeed, thanks for pointing it out. Resolved by dropping the first commit and applying a minimal diff to `netbase.cpp`.

💬 maflcko commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767054547)
Not sure about this. `tfm::format` is the only remaining function preventing the removal of `./test/lint/lint-format-strings.py`. I wanted to do this in a follow-up, but if this function is renamed, it will be harder, because all call sites will have to be renamed.

My preference would be to leave the name as-is and instead just delete the unchecked one, or rename that one to `format_maybe_throw` (or similar).

If you don't mind, it would be good to delete `./test/lint/lint-format-strings.py
...

💬 stickies-v commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1767054801)
I don't agree it's redundant:
- it's not obvious from the tests that we're not just checking error messages, but that we're ensuring no errors are thrown, which is the difference with `tfm::format()`
- additional tests to try_format may be added in the future that test other aspects, so this acts as a little delineator

💬 mzumsande commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2361367099)
Though one could also make a case for not allowing importdescriptor/rescan during background sync, with the reasoning "just wait a little bit longer" that doesn't really apply to the pruning case. Would be interesting to know if this has ever been discussed for assumeutxo.

🚀 fanquake merged a pull request: "Follow-up after AutoFile position caching: remove unused code"
(https://github.com/bitcoin/bitcoin/pull/30927)

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2361384689)
Thanks for the reviews! Added a commit renaming the file build.h to bitcoin-build-info.h, to account for both the namespacing suggestions (https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2310630029) and the request for a more descriptive name (https://github.com/bitcoin/bitcoin/pull/30856#discussion_r1764891407). Let me know if another name is preferred (dashes vs. underscores? put "git" into filename or not?) and if a scripted-diff for moving bitcoin-config.h is preferred in thi
...

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#discussion_r1767071727)
> Now that HAVE_BUILD_INFO is removed, this can be placed under the "normal" includes, no?

I left it where it is for now, as I thought it make sense if the comments and the preprocessor commands evaluating `BUILD_GIT_TAG` are immediately below, but no hard feelings. Happy to move only the include up if that is preferred.

👍 maflcko approved a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2315960896)
lgtm

Maybe squash, to avoid changing the file name twice?

💬 theStack commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2361420302)
> Maybe squash, to avoid changing the file name twice?

Makes sense yeah, done.

🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2315917440)
re: https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2361045807

Thanks for the review!

> The only part that seems (slightly) more involved is in [00a82f4](https://github.com/bitcoin/bitcoin/commit/00a82f45004a887c2b58ed8da3db9280e0ec8b5c). If someone were to refactor `BlockValidationState`, they would have to also adjust its `CustomBuildMessage` and `CustomReadMessage` counterparts.

I think it might be good idea to change `testBlockValidity` method anyway to return validation
...

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1767053902)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1766639028

> [974e041](https://github.com/bitcoin/bitcoin/commit/974e041b794072395e41701cd7a71ebdcc362e48): `serialize.h` already defines a `concept Unserializable` and `Serializable `, introduced by @maflcko in #29056.

This is interesting. The concepts are not interchangeable because the definitions in #29056 check whether a particular class can be serialized and unserialized with a particular stream class, while the concepts h
...

💬 kevkevinpal commented on pull request "test: Remove 0.16.3 test from wallet_backwards_compatibility.py":
(https://github.com/bitcoin/bitcoin/pull/30920#issuecomment-2361475900)
was there a seed value for https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2357565564?

I'm trying to look for it to try and reproduce but am having trouble finding it

👍 theuni approved a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2316048259)
utACK 7025942687fd5e91d0a10ce5b2ac673b67a63491

📝 jonatack opened a pull request: "netinfo: add peer services column and outbound-only peers list"
(https://github.com/bitcoin/bitcoin/pull/30930)
Been using this since May 2023.

- add a peer services column (considered displaying the p2p_v2 flag as "p" or "2"; proposing "2" here for continuity with the "v" column)
- remove the "v" transport protocol column, as that info is provided in the new peer services column
- clarify in the help that "relaytxes" and "addr_relay_enabled" are from getpeerinfo
- add a level 5 for an outbound-only peer list. Several people have requested this, to keep the output within screen limits when running n
...

💬 brunoerg commented on pull request "fuzz: speed up addrman":
(https://github.com/bitcoin/bitcoin/pull/30688#issuecomment-2361513087)
Just provided the fuzz inputs for testing: https://github.com/brunoerg/qa-assets/pull/1