💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1138541531)
Hmm i thought i was following the style used elsewhere, but i must have dreamed it. Done.

💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1138548620)
Hmm that's a good point re unsigned type, but it needs to be able to return negative values. So i guess it's up to the caller to make sure they don't call this with `b > a` if `a` is unsigned, as with regular substractions?

💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1471855629)
Rebased past #27153.

💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1471858282)
Rebased past #27153.

💬 stickies-v commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#discussion_r1137600236)
Given that there is only a single non-test callsite of `FindByte()`, would it make sense to just update the fn signature to take `std::byte` directly?

💬 stickies-v commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#discussion_r1138522337)
I think some of the rationale of this implementation should be in the docs so future contributors don't simplify the code again to unintentionally undo the performance gains, e.g. why the modulo operator is kept outside of the while loop seems quite important and non-trivial?

💬 stickies-v commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#discussion_r1137601031)
nit: and a bunch more of those
```suggestion
size_t buf_offset{m_read_pos % vchBuf.size()};
```

💬 stickies-v commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#discussion_r1138558337)
Unrelated to this PR, but I think it would also be helpful to improve the docs to specify that if `ch` is not found, `std::ios_base::failure` is thrown (from `Fill()`). It's an essential part of the interface imo.

Perhaps worth improving on this behaviour, and have `FindByte()` throw its own error, by wrapping the `Fill()` command in a try/catch? Orthogonal to this PR, though. (And I also don't like that we're [catching a general `std::exception`](https://github.com/bitcoin/bitcoin/blob/ebb15
...

👍 vasild approved a pull request: "p2p: return `CSubNet` in `LookupSubNet`"
(https://github.com/bitcoin/bitcoin/pull/26078)
ACK 4e377d8b684218d49229babc5959e07f38793549

💬 1440000bytes commented on issue "Permission to comment on PR 27235":
(https://github.com/bitcoin/bitcoin/issues/27243#issuecomment-1471914595)
> and yet he's perfectly free to open up a new PR with the same set of changes and comments all over it...
>
> censorship doesn't exist

Thanks for the suggestion. I will follow these in the future for PRs trying to avoid reviews:

1. Open a PR with same commit and add comments
2. If 1 doesn't work. Send email to bitcoin-core-dev@lists.linuxfoundation.org
3. If 2 doesn't work. Use an invite only [service](https://i.imgur.com/HUGS3gG.png) to post anonymous reviews

💬 vasild commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#issuecomment-1471954276)
ACK 05eeba2c5fb312e0e6a730b01eb7d1b422d75dbb

🚀 fanquake merged a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(https://github.com/bitcoin/bitcoin/pull/26177)

💬 mxaddict commented on pull request "Updated copyright years":
(https://github.com/bitcoin/bitcoin/pull/27267#issuecomment-1472076871)
> The tool is detailed here, although it doesn't mention when it's actioned: https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#copyright_headerpy

Thanks :+1:

📝 mxaddict opened a pull request: "Fixed a warning from python linter"
(https://github.com/bitcoin/bitcoin/pull/27268)
null

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138685543)
Would be nice to extend the comment to explain what is the `bool` in the return value.

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138744578)
Add `AssertLockNotHeld(m_msg_process_queue_mutex)` as per `doc/developer-notes.md`:
> - Combine annotations in function declarations with run-time asserts in
function definitions:

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138747571)
The nested scope is unnecessary now, you can remove `{` and `}`.

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138756727)
`s/Send/Sent/`
`s/received_bytes/sent_bytes/`

```suggestion
void AccountForSentBytes(const std::string& msg_type, size_t sent_bytes)
```

💬 SkybuckFlying commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472092677)
What is the difference between the two ? confused...

💬 hebasto commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472095887)
> What is the difference between the two ? confused...

Is your question about screenshots or about websites?