💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1916279728)
131eb4e630f958f1b5330bb9d170716c33655880: this should be a constant, not part of `BlockCreateOptions`.

E.g. `MINIMUM_BLOCK_RESERVED_WEIGHT{2000}` defined next to `DEFAULT_BLOCK_MAX_WEIGHT`.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1916278786)
131eb4e630f958f1b5330bb9d170716c33655880: sane -> safety

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1916275287)
9990552514328da45c19ed294c5bc79562aa12bc nit: "including"

✅ fanquake closed an issue: "The 28.1 release is not tagged in github"
(https://github.com/bitcoin/bitcoin/issues/31660)

💬 fanquake commented on issue "The 28.1 release is not tagged in github":
(https://github.com/bitcoin/bitcoin/issues/31660#issuecomment-2592293287)
I've published a release: https://github.com/bitcoin/bitcoin/releases/tag/v28.1.

👍 stickies-v approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2547086177)
ACK fa51381790fe19e37e04a01a39cc94a00dcc441c - replacing linters with compile time checks is a very nice win. The newly introduced `TranslatedLiteral` and `BilingualFmt` make for a nice interface, and `RuntimeFormat` preserves the ability to bypass compile-time checks in a clear and convenient way.

A couple of non-blocking nits (sorry, I thought I'd posted them already) that shouldn't hold up this PR.

💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1913420814)
in fad276ad6518a394d065943d458e8c7cce3f997c

nit: could-reuse `TranslateFn`:

<details>
<summary>git diff on fad276ad65</summary>

```diff
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 5c5965245b..1b2d97faff 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -50,7 +50,7 @@ using util::ToString;
// just use a plain system_clock.
using CliClock = std::chrono::system_clock;

-const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
+co
...

💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1913646741)
Yeah it's fine as is too. The change I had in mind was just adding `RunTimeWrapped-wrapped` in case users just read the docstring and don't understand why their std::string argument fails.

> // Added for Bitcoin Core. Wrapper for checking format strings at
// compile time. Unlike ConstevalFormatString this supports
// RunTimeFormat-wrapped std::string for runtime string formatting
// without compile time checks.

💬 stickies-v commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1913600974)
style nit: using `internal` function overloads seems a bit more readable to me, but up to you what you prefer

<details>
<summary>git diff on fa93b05a4d</summary>

```diff
diff --git a/src/util/translation.h b/src/util/translation.h
index 27747a16f2..347bc5151d 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -81,29 +81,26 @@ consteval auto _(util::TranslatedLiteral str) { return str; }
/** Mark a bilingual_str as untranslated */
inline bilingual_str Untranslated
...

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1916382498)
thx, added a commit

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1916382700)
leaving as-is for now

💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1916383028)
thx, pushed doc fixup

👍 stickies-v approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2552286452)
re-ACK fa3efb5729091a36a0e82316e9e4b7c09115dc2e

No changes except for consistent re-use of `TranslateFn`, minor docstring change and un-inline-ing test translate function.

🤔 Sjors reviewed a pull request: "Split CConnman"
(https://github.com/bitcoin/bitcoin/pull/30988#pullrequestreview-2552280742)
> > ... dropping `AddSocketPermissionFlags`. Can you move that to a separate (earlier?) commit?
>
> Done.

Thanks. Don't forget to drop "Also, separate the listening socket from the permissions" from the commit description of e5d36eea015efc31aa38d540af4cf39c9e2e46b0.

Along similar lines, though less important, you could also drop `ListenSocket` and first introduce `m_listen` as a member of `CConnman`, before moving it to `Sockman`.

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1916382790)
fd81820214e695ba228a954506397c3d781fe3fe: do want to add an `Assume` here, given that `Bind` always adds an entry?

✅ stickies-v closed a pull request: "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error"
(https://github.com/bitcoin/bitcoin/pull/30928)

💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2592463052)
Closing this PR. Thanks to related PRs like #31174, #31072 and #31061, tfm::format error throwing should be significantly reduced, so there's increasingly little point in making this behaviour change.

🤔 rkrux reviewed a pull request: "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor"
(https://github.com/bitcoin/bitcoin/pull/31590#pullrequestreview-2552298207)
Concept ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424

nit in pr title:

```diff
- "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor"
+ "descriptors: Try pubkeys of both parities when retrieving the private keys for an xonly pubkey in a descriptor"
```

💬 rkrux commented on pull request "descriptors: Try pubkeys of both evenness when retrieving the private keys for an xonly pubkey in a descriptor":
(https://github.com/bitcoin/bitcoin/pull/31590#discussion_r1916393178)
Why does `GetPrivKey` of `ConstPubkeyProvider` not end up using the `pos` argument? Is it because it's supposed to be a "constant" provider?

📝 fanquake reopened a pull request: "depends: Qt 5.15.16"
(https://github.com/bitcoin/bitcoin/pull/30774)
Contains a handful of miscellaneous bug fixes.
We can drop a few of our patches.

See https://github.com/qt/qtbase/compare/v5.15.14-lts-lgpl...v5.15.16-lts-lgpl.