🚀 achow101 merged a pull request: "rpc: Fix invalid bech32 address handling"
(https://github.com/bitcoin/bitcoin/pull/27727)
(https://github.com/bitcoin/bitcoin/pull/27727)
💬 achow101 commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1561508601)
ACK ac8d72668c7bbd9f8771442fe25fd37f08c3f5ae
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1561508601)
ACK ac8d72668c7bbd9f8771442fe25fd37f08c3f5ae
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204429122)
cce96182ba2457335868c65dc16b081c3dee32ee: the goal of the above loop seems to be to find one entry: the `vBlocksInFlight` entry for this particular block and `from_peer`. A followup could move that into its own function, and/or have the subsequent operations on the entry live _outside_ the loop.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204429122)
cce96182ba2457335868c65dc16b081c3dee32ee: the goal of the above loop seems to be to find one entry: the `vBlocksInFlight` entry for this particular block and `from_peer`. A followup could move that into its own function, and/or have the subsequent operations on the entry live _outside_ the loop.
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204472811)
It ended up in 03423f8bd12b95a06a4a9d8377e781625dd38aae
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204472811)
It ended up in 03423f8bd12b95a06a4a9d8377e781625dd38aae
💬 achow101 commented on pull request "test: Disable legacy wallet for mempool_packages.py":
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1561578795)
Is there a reason that this test needs the wallet in the first place?
(https://github.com/bitcoin/bitcoin/pull/27735#issuecomment-1561578795)
Is there a reason that this test needs the wallet in the first place?
💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1204513106)
> -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF to reduce CPU?
These two control the generation of build targets, however nothing should be getting compiled, as `LLVM_BUILD_TESTS` and `LLVM_BUILD_BENCHMARKS` [both default to `OFF`](https://llvm.org/docs/CMake.html). However I've also added some additional `-DLLVM_INCLUDE_*` options that should reduce compilation.
> -DLLVM_USE_LINKER=lld and/or -DLLVM_PARALLEL_LINK_JOBS=1 to reduce change of OOM?
I could follow up with a change
...
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1204513106)
> -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF to reduce CPU?
These two control the generation of build targets, however nothing should be getting compiled, as `LLVM_BUILD_TESTS` and `LLVM_BUILD_BENCHMARKS` [both default to `OFF`](https://llvm.org/docs/CMake.html). However I've also added some additional `-DLLVM_INCLUDE_*` options that should reduce compilation.
> -DLLVM_USE_LINKER=lld and/or -DLLVM_PARALLEL_LINK_JOBS=1 to reduce change of OOM?
I could follow up with a change
...
💬 achow101 commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#discussion_r1204518144)
AFAIK there aren't any other external signer software yet.
(https://github.com/bitcoin/bitcoin/pull/24313#discussion_r1204518144)
AFAIK there aren't any other external signer software yet.
💬 achow101 commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1561612282)
ACK e3d02f8ef8cfe1d3c502ede863d14f82212a0ce6
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1561612282)
ACK e3d02f8ef8cfe1d3c502ede863d14f82212a0ce6
👋 brunoerg's pull request is ready for review: "net, refactor: net_processing, add `ProcessCompactBlockTxns`"
(https://github.com/bitcoin/bitcoin/pull/26969)
(https://github.com/bitcoin/bitcoin/pull/26969)
💬 instagibbs commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561640287)
@ajtowns
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1561640287)
@ajtowns
💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1204534315)
updated test and also included warning in release note
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1204534315)
updated test and also included warning in release note
🤔 Sjors reviewed a pull request: "Parallel compact block downloads, take 3"
(https://github.com/bitcoin/bitcoin/pull/27626#pullrequestreview-1442325175)
Post merge utACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83 modulo question about missing `return`. Other comments can wait for a followup.
Conceptually this PR still relies on our node immediately reacting to an incoming compact block message. This forces us to be a bit impatient and potentially use more bandwidth than needed.
In a followup we could instead track which peers have announced a block and then request block transactions from other peers if the first peer takes too long. This co
...
(https://github.com/bitcoin/bitcoin/pull/27626#pullrequestreview-1442325175)
Post merge utACK d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83 modulo question about missing `return`. Other comments can wait for a followup.
Conceptually this PR still relies on our node immediately reacting to an incoming compact block message. This forces us to be a bit impatient and potentially use more bandwidth than needed.
In a followup we could instead track which peers have announced a block and then request block transactions from other peers if the first peer takes too long. This co
...
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204480914)
03423f8bd12b95a06a4a9d8377e781625dd38aae This deleted comment line is still relevant when deleting the request:
```cpp
// Forget about all prior requests. If a block was already in-flight
// for a different peer, its BLOCKTXN response will be dropped.
```
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204480914)
03423f8bd12b95a06a4a9d8377e781625dd38aae This deleted comment line is still relevant when deleting the request:
```cpp
// Forget about all prior requests. If a block was already in-flight
// for a different peer, its BLOCKTXN response will be dropped.
```
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204478988)
03423f8bd12b95a06a4a9d8377e781625dd38aae: some of the above deleted comment is still relevant.
```cpp
// If the peer does not send us a block, vBlocksInFlight remains non-empty,
// causing us to timeout and disconnect.
```
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204478988)
03423f8bd12b95a06a4a9d8377e781625dd38aae: some of the above deleted comment is still relevant.
```cpp
// If the peer does not send us a block, vBlocksInFlight remains non-empty,
// causing us to timeout and disconnect.
```
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204497452)
03423f8bd12b95a06a4a9d8377e781625dd38aae could add a static assert `MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK > 1` in a followup.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204497452)
03423f8bd12b95a06a4a9d8377e781625dd38aae could add a static assert `MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK > 1` in a followup.
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894)
03423f8bd12b95a06a4a9d8377e781625dd38aae: why not `return`?
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204491894)
03423f8bd12b95a06a4a9d8377e781625dd38aae: why not `return`?
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204506528)
03423f8bd12b95a06a4a9d8377e781625dd38aae: "Give up" implies that this peer could have done something to prevent this, but that's often not the case (`Nodes sending cmpctblock messages SHOULD limit prefilledtxn to 10KB of transactions.` - BIP152 footnote 5).
```cpp
// Do not followup with `GETBLOCKTXN` to this peer,
// wait for an outbound peer to announce the compact block instead.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204506528)
03423f8bd12b95a06a4a9d8377e781625dd38aae: "Give up" implies that this peer could have done something to prevent this, but that's often not the case (`Nodes sending cmpctblock messages SHOULD limit prefilledtxn to 10KB of transactions.` - BIP152 footnote 5).
```cpp
// Do not followup with `GETBLOCKTXN` to this peer,
// wait for an outbound peer to announce the compact block instead.
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204514939)
d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83: maybe explain what's going on here. We send an unsolicited compact block from these peers to our node. Because the block adds to our tip the message doesn't get ignored. Once the block is successfully processed we mark the peer as high bandwidth. This is done on a first-come-first-serve basis. This results in the `[False, True, True, True]` pattern below.
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204514939)
d7f359b35e8b1e9acc4d397de262cd9ba9bbcb83: maybe explain what's going on here. We send an unsolicited compact block from these peers to our node. Because the block adds to our tip the message doesn't get ignored. Once the block is successfully processed we mark the peer as high bandwidth. This is done on a first-come-first-serve basis. This results in the `[False, True, True, True]` pattern below.
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204495961)
03423f8bd12b95a06a4a9d8377e781625dd38aae "on failure" -> "when missing transactions" (the word failure is ambiguous, this is different from `READ_STATUS_FAILED`)
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204495961)
03423f8bd12b95a06a4a9d8377e781625dd38aae "on failure" -> "when missing transactions" (the word failure is ambiguous, this is different from `READ_STATUS_FAILED`)
💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561670353)
> This is really cool and I'd love to have it as an option in Bitcoin Core. However, most CoinJoin implementations are more advanced in this regard – for example, WabiSabi performs both divisions and consolidations. And it makes me wonder to which extent it's possible to decentralize coordination to the point where clients randomly take turns in doing it.
>
Yes, CoinJoin is definitely more powerful, however it has some drawbacks.
In particular, if it's possible to detect that an UTXO has
...
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1561670353)
> This is really cool and I'd love to have it as an option in Bitcoin Core. However, most CoinJoin implementations are more advanced in this regard – for example, WabiSabi performs both divisions and consolidations. And it makes me wonder to which extent it's possible to decentralize coordination to the point where clients randomly take turns in doing it.
>
Yes, CoinJoin is definitely more powerful, however it has some drawbacks.
In particular, if it's possible to detect that an UTXO has
...