💬 sdaftuar commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1572728184)
> Namely, high-feerate evictated/replaced chunks of transactions could be kept in a buffer of transactions ordered by their last descendants outputs. When a new package comes in, in case of missing parents outputs, a lookup is realized in the buffer to see if the package attached to evicted/replaced chunks ancestor-feerate is above the cluster spending the same utxo or above the bottom mempool chunks.

[snip additional comments related to this]

If I'm understanding your post correctly, you'
...

👋 achow101's pull request is ready for review: "rpc: Be able to access RPC parameters by name"
(https://github.com/bitcoin/bitcoin/pull/27788)

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1213654672)
Great, done!

💬 hebasto commented on pull request "guix: fix CURL disable flag in osslsigncode":
(https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1572754794)
> It is not clear why this is a fix. `CMAKE_DISABLE_FIND_PACKAGE_CURL` is a valid [variable](https://cmake.org/cmake/help/latest/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html) that:
>
> > can be used to build a project without an optional package

Considering the `FindCURL` module [source code](https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/FindCURL.cmake), I believe that the current using of the `CMAKE_DISABLE_FIND_PACKAGE_CURL` variable is optimal.

FWIW, the build
...

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1213672054)
I didn't touch `ComputeAndSetWaste` parameters value, perhaps who did it copied (?) from: https://github.com/bitcoin/bitcoin/blob/34ac3f438a268e83af6cd11e2981e5bc07f699c9/src/wallet/coinselection.cpp#L179

I'm going to change it to make it more "realistic"/robust.

💬 kevkevinpal commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1572777861)
Concept ACK

💬 furszy commented on pull request "streams: Drop confusing DataStream::Serialize method and << operator":
(https://github.com/bitcoin/bitcoin/pull/27800#discussion_r1213677599)
does this need to be a Span? Wouldn't be the same to just pass `finalAlert`?

💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1572800909)
Re https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1572503569

> The node's KernelNotifications handler could treat this just like a fatal error to keep current behavior the same. Other applications using libbitcoinkernel might choose to handle the error in a different way for example by sending a notification or trying to free up disk space, rather than shutting down the service right away.

Is there really a qualitative difference between the errors? Instead of adding multiple f
...

💬 kevkevinpal commented on pull request "wallet: Add tracing for sqlite statements":
(https://github.com/bitcoin/bitcoin/pull/27801#issuecomment-1572808184)
ACK [f45e066](https://github.com/bitcoin/bitcoin/pull/27801/commits/f45e0669dd1f1bdb243e645691e44993905a3919) I tested by switching to this commit and doing the following
```
make -j"$(($(nproc)+1))"
./src/bitcoind -regtest -debug=walletdb -loglevel=walletdb:trace
```
In different window
```
./src/bitcoin-cli -regtest createwallet "node2"
```

Then you can observe the bitcoind logs
example of one
```
2023-06-01T21:14:59Z [/Users/<my path>/wallet.dat] SQLite Statement: INSERT or REP
...

🤔 furszy reviewed a pull request: "streams: Drop confusing DataStream::Serialize method and << operator"
(https://github.com/bitcoin/bitcoin/pull/27800#pullrequestreview-1456346592)
lgtm ACK 5cd0717a

💬 ryanofsky commented on pull request "streams: Drop confusing DataStream::Serialize method and << operator":
(https://github.com/bitcoin/bitcoin/pull/27800#discussion_r1213705810)
> does this need to be a Span? Wouldn't be the same to just pass `finalAlert`?

Because `finalAlert` is a vector of bytes, serializing the vector would first write the number of bytes in the vector to the stream, followed by the bytes themselves. Since spans unlike vectors are fixed length, serializing a span just writes the raw bytes to the stream without a length prefix.

💬 achow101 commented on pull request "[24.x] rpc: Fix invalid bech32 handling":
(https://github.com/bitcoin/bitcoin/pull/27755#issuecomment-1572821465)
ACK c2e9214effe9abecae6f81cb10158f9661065da3

Verified that the additional changes are the minimal required to make the new test work.

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1213710664)
Done

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1572825952)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1213485109 and https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1213479178

🚀 achow101 merged a pull request: "[24.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27755)

💬 achow101 commented on pull request "[23.x] rpc: Fix invalid bech32 handling":
(https://github.com/bitcoin/bitcoin/pull/27756#issuecomment-1572828786)
ACK d98770720885cdce1bbe2109a6d214c63fa1814a

Diff matches #27755

🚀 achow101 merged a pull request: "[23.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27756)

💬 ryanofsky commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1572835701)
> Is there really a qualitative difference between the errors?

I didn't dig into the actual errors, but the qualitative difference here seems to be that the other AbortNode errors actually need to be fatal and should try to shut down to prevent getting into a worse state. But an error flushing state in memory to disk does not need to be fatal, as long the state in memory is kept and can be saved later. There are lots of ways an application could handle a failed flush notification, and I think
...

💬 furszy commented on pull request "streams: Drop confusing DataStream::Serialize method and << operator":
(https://github.com/bitcoin/bitcoin/pull/27800#discussion_r1213724380)
ah ok. I mixed up stuff by reading the `CVectorWriter` class description -> "Minimal stream for overwriting and/or appending to an existing byte vector" (it doesn't mention the serialization..). I should have checked the constructor. My bad, thanks.

💬 furszy commented on pull request "Introduce 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1572850482)
title and description updated.
And also went a bit further with the `rpc_getblockfrompeer.py` changes, added some explanations to it.