💬 glozow commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236928375)
Oops. ACK faed5d3870f

🤔 glozow reviewed a pull request: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453#pullrequestreview-2186272759)
ACK faed5d3870f

💬 fanquake commented on pull request "depends: build FreeType with CMake":
(https://github.com/bitcoin/bitcoin/pull/29880#issuecomment-2236937905)
Guix Build (x86_64, aarch64):
```bash
bd6c916cc5fdd322bf9e7e3cecbe45a68551c0135d63b35709b32fccde289834 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/SHA256SUMS.part
12c161c6b94ccb15d420fe27f7a812c95247a2614db5f8c6e666e04b5ef0a7d3 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu-debug.tar.gz
99c6d258715f38c7b56eefd0b08e80552012e8903f26d28225b690f432839545 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu.tar.gz
...

💬 Sjors commented on pull request "Introduce waitFeesChanged() mining interface":
(https://github.com/bitcoin/bitcoin/pull/30443#issuecomment-2236950771)
@sdaftuar @sipa I made a branch that combines #28676 with #29432: https://github.com/Sjors/bitcoin/tree/sv2-cluster

The main commit is https://github.com/bitcoin/bitcoin/commit/f646977ddac0622889d484683ae59c262d50d8dc. It slightly changes the `waitFeesChanged()` interface. The implementation now frequently creates a new template.

The Template Provider then uses that to see if sending a new `NewTemplate` message is justified.

It still takes 20-30ms on mainnet to create a template, withou
...

👍 ryanofsky approved a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2186195988)
Code review ACK c3a9c71c7077324bf0aa40f834f7537a14619340

I think the changes are all good, but the PR and the most important commit "fix: Make TxidFromString() respect string length" (c3a9c71c7077324bf0aa40f834f7537a14619340) are lacking a good description that actually says what the bug is.

I think they should say that currently `TxidFromString` takes a string_view parameter, but internally it is calling the `uint256S` function which expects a nul-terminated string. If `TxidFromString` is
...

💬 ryanofsky commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683055614)
In commit "refactor: Change base_blob::SetHex() to take std::string_view" (f77d961d926405637dfbdfb9a9baea1fab4f1b7b)

Note: This line isn't directly equivalent to previous code because it trims whitespace from the end of the string, not just the beginning, but the net effect is the same because any whitespace at the end of the string would be ignored anyway.

I'm surprised looking at the implementation of SetHex to see how unreliable it is in general. Since it can't report errors it winds up
...

💬 ryanofsky commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1683086730)
In commit "refactor: Change base_blob::SetHex() to take std::string_view" (f77d961d926405637dfbdfb9a9baea1fab4f1b7b)

It seems not great, especially in the light of the unterminated string bug, that this will iterate over the entire string when only the beginning portion of the string may be needed. For example, if the string is 1MB and the blob is 32 bytes, this will iterate over 1MB. Would changing this to:
```c++
while (digits < str.size() && digits < WIDTH*2 && ::HexDigit(str[digits]) !=
...

🚀 glozow merged a pull request: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453)

💬 glozow commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2237009418)
post merge ACK

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683160882)
Of course

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683164631)
Ah, my mistake, this was an intermediary commit - fine either way

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2237035374)
ACK d7f94e0fa9ec3641405e6909ab7da4e48865e840

👍 itornaza approved a pull request: "crypto, refactor: add new KeyPair class"
(https://github.com/bitcoin/bitcoin/pull/30051#pullrequestreview-2186375491)
re tACK 5e9f4f41de98c2cdcc74e31fe925b2f5cf9f6aec

Thank you for your commit redo in a more granular way. Just a couple of questions really in the discussion above. To be thorough, rebuilt and retested the commit and all tests included the extended suite pass.

🤔 glozow reviewed a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2186380821)
reACK 172c1ad026cc38c6f52679e74c14579ecc77c48e

💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#discussion_r1683190037)
Measuring exec/s without lazy init:


With:


💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#discussion_r1683143984)
There are more message types that are allowed after the version message and before the verack (some of them are also allowed post verack), essentially all of the ones processed in [this block](https://github.com/bitcoin/bitcoin/blob/20ccb30b7af1c30cece2b0579ba88aa101583f07/src/net_processing.cpp#L3894-L4061) (`wtxidrelay`, `sendaddrv2`, `sendtxrcncl`, `sendheaders`, `sendcmpct`).

I used `ALL_NET_MESSAGE_TYPES`, so that we can avoid touching this test if we add more handshake related message t
...

💬 dergoegge commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#discussion_r1683123261)
My idea was that this might help a fuzzer to more realistically simulate how time would change between messages. Setting it directly would of course also work but I thought it might result in more mutations that are completely bogus (the time isn't attacker controlled so this seemed less useful). Happy to change it, probably doesn't make a huge difference. What do you think?

💬 Sjors commented on pull request "Introduce waitFeesChanged() mining interface":
(https://github.com/bitcoin/bitcoin/pull/30443#issuecomment-2237077920)
Rebased after #30356 landed. Changed `fees_before` to be a reference to `CAmount` based on the above experience of using it.

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2237085603)
Rebased after #30356 (`#include` order).

💬 Sjors commented on pull request "guix: bump time-machine to e6e70abe65850da0ae69428654375d367088ef5e":
(https://github.com/bitcoin/bitcoin/pull/30452#issuecomment-2237090936)
guix hashes:

```
365b4e5e4f876a949834d6f444edb079851e7e77b4fc77f7d6bafd1958f87a10 guix-build-314407a2fb80/output/aarch64-linux-gnu/SHA256SUMS.part
d8f7f8a9d84e37df272a53e5a69f4325196dfc7d43b1a2c12415e81a140f80ad guix-build-314407a2fb80/output/aarch64-linux-gnu/bitcoin-314407a2fb80-aarch64-linux-gnu-debug.tar.gz
aaafb96f75597be1206329b5632e32d1f844e15e69415184e8aada3397789bc6 guix-build-314407a2fb80/output/aarch64-linux-gnu/bitcoin-314407a2fb80-aarch64-linux-gnu.tar.gz
d3eec1297168c6104
...