💬 brunoerg commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1213614742)
I suppose it bypass the `maxconnection`, does it? If so, it worths to update the `-maxconnections` mentioning it.

💬 Xekyo commented on issue "fuzz: mini_miner: Timeout in mini_miner":
(https://github.com/bitcoin/bitcoin/issues/27799#issuecomment-1572704415)
Yeah, it’s a grandfather paradox, in this fuzz seed, a transaction is it’s own great-grandparent. Looking into the fix.

💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1213631522)
Thank you for taking a look!

> So it seems to me the approach here is not very friendly to potential libbitcoinkernel applications, and is not even very friendly to our own application since it exposes the fRequestShutdown global more broadly and makes it harder to get rid of.

I was trying this out today on my proof of concept branch and got the same impression. It is unwieldy and the naming is bad.

> I think a better approach would be to get rid of the fRequestShutdown global and repla
...

💬 achow101 commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#issuecomment-1572723855)
ACK 1a27bec1e9e40166d6ff7f0492115107289e7609

💬 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