💬 rkrux commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1837885827)
A nit but it's easier to read as well, catches eyes quickly.

```suggestion
'code': -342, 'message': f'Cannot decode response in UTF-8 format, content: {data}, exception: {e}'})
```

💬 rkrux commented on pull request "test: report detailed msg during utf8 response decoding error":
(https://github.com/bitcoin/bitcoin/pull/31251#discussion_r1837891200)
`content: {data}`

Out of curiosity: Do you think we should limit printing the data here to a certain length or printing all of it is fine as well? Idk atm if the RPC responses from core have a size limit.

🚀 fanquake merged a pull request: "test: Add combinerawtransaction test to rpc_createmultisig"
(https://github.com/bitcoin/bitcoin/pull/31249)

💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2470244740)
@m3dwards thanks! I'll give that a try. Any idea what was wrong with the syntax I used?

👋 Sjors's pull request is ready for review: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)

💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2470265299)
That worked! See https://github.com/Sjors/bitcoin/pull/70/checks where the "push" actions are skipped.

📝 fanquake opened a pull request: "guix: scope pkg-config to Linux only"
(https://github.com/bitcoin/bitcoin/pull/31276)
After #31181, `pkg-config` is no-longer needed for macOS or Windows Guix builds. It's still needed for Linux, as it's used by a Qt subdependency (fontconfig to find freetype). However we should also no-longer need it for Qt itself, when building using depends.

💬 Sjors commented on issue "v27.2 guix build fails with hash mismatch":
(https://github.com/bitcoin/bitcoin/issues/31266#issuecomment-2470288243)
I ended up working around the issue by downloading the file elsewhere:

```sh
guix download https://www.gnupg.org/ftp/gcrypt/gnutls/v3.8/gnutls-3.8.1.tar.xz
```

After that `guix build` is happy.

My guess is that `.xy` files have been purged from the internet left and right. Not sure if this is worth filing an upstream issue.

💬 ryanofsky commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1837939655)
In commit "coins, refactor: Make AddFlags, SetDirty, SetFresh static" (d5e3bbf440aa948df8159999fd4eb5275c354b93)

It would make sense to rename `self` to `pair` this commit instead of in later commit afb0a3c88691cadc07fc985483f4cae1f486f6be so this code does not have to change twice and so later the change in the later commit can be move-only. Right now the later commit is tedious to review (or at least I don't know an easy way to review it) because the argument is renamed at the same time as
...

👍 ryanofsky approved a pull request: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906#pullrequestreview-2429297641)
Code review ACK 9edbe5a48fe9a2c0d364882fcb7d44ae6aac6d24. Only small changes since last review like adding an Assume, changing commit messages, renaming self to pair. Thanks for the updates!

I left two suggestions which are not important and don't affect the final code, just meant to make review a little easier for the next reviewer. Feel free to ignore them or save for a later update.

💬 ryanofsky commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1837922879)
re: https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1829401813

> Wouldn't I need to make `AddFlags` static as well in that case?

No that should not be necessary, and wouldn't suggest that. The suggestion is to make the new `SetFresh` and `SetClean` methods static and have them call `self.second.AddFlags` instead of `AddFlags` in this commit so all the new calls to these methods don't need to change again in commit d5e3bbf440aa948df8159999fd4eb5275c354b93.

👍 ryanofsky approved a pull request: "Prune mining interface"
(https://github.com/bitcoin/bitcoin/pull/31196#pullrequestreview-2429358194)
Code review ACK a67ba64f8f63a0f9f2c86d72cf7240bee7c5388c. Only changes since last review were a tweak to fix a failing test, and a commit message update.

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1837989979)
Yeah, adding `mempool.m_opts.require_standard` to the if condition would make sense - `acceptnonstdtxn` should turn off all dust checks including this one imo

🤔 ryanofsky reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2429375066)
Updated ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba -> fe39acf88ff552bfc4a502c99774375b91824bb1 ([`pr/tcheck.4`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.4) -> [`pr/tcheck.5`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/tcheck.4..pr/tcheck.5)) with suggested changes. Thanks for the reviews and suggestions!

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837966979)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950

> As you say, only the length and type are no longer checked. This PR now implements rudimentary checking of flags, width and precision.

Thanks, applied suggestion

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837996485)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835766554

> we might as well extract number parsing to a local lambda like you did with the other ones.

This seems reasonable but I"m not sure it's clearer, and it does make the diff bigger replacing the `maybe_num` code that doesn't have to change currently. Happy to apply this suggestion if other reviews think it is a good idea.

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837973744)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834499010

Thanks, added

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837974075)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834504076

Thanks, added

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837983669)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770704

I can apply the `"#0- +"sv.find` change if others like it, but to me it seems less readable and only a little shorter.

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837980148)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770569

Nice suggestion! Applied