Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 fanquake commented on issue "seeds: seed.bitcoin.jonasschnelli.ch not returning results":
(https://github.com/bitcoin/bitcoin/issues/32590#issuecomment-2918785885)
I am seeing this again.
💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2113587244)
Is that possible without adding a new argument for single strings?
💬 fanquake commented on pull request "build: Add resource file and manifest to `bitcoin.exe`":
(https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918864171)
Guix Build:
```bash
0762b5cbe6135d2e83b80f96248be668bb5ac514511b07f59fbd1df5094d9eee guix-build-dbb2d4c3d547/output/aarch64-linux-gnu/SHA256SUMS.part
c65f1e1fe6feb729acd0b987066d7131e5e7220c1e45c6731f9e46e1b2dca187 guix-build-dbb2d4c3d547/output/aarch64-linux-gnu/bitcoin-dbb2d4c3d547-aarch64-linux-gnu-debug.tar.gz
05e4220765d4c85c5bfe89392e264b0c0cd6bd093fed2eb025f33cd9180f220c guix-build-dbb2d4c3d547/output/aarch64-linux-gnu/bitcoin-dbb2d4c3d547-aarch64-linux-gnu.tar.gz
e0bb9020320a12ca
...
🚀 fanquake merged a pull request: "build: Add resource file and manifest to `bitcoin.exe`"
(https://github.com/bitcoin/bitcoin/pull/32634)
📝 l0rinc opened a pull request: "blocks: force hash validations of blocks read from disk explicit"
(https://github.com/bitcoin/bitcoin/pull/32638)
A follow-up to https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094072165, after which validating the hash of a read block from disk doesn't incur the cost of calculating its hash anymore.

### Summary

This PR adds explicit checks that the read block header's hash matches the one we were expecting.

### Context

After the previous PR, validating a block's hash during read operations became essentially free. This PR leverages that by requiring callers to provide a block's expec
...
🚀 fanquake merged a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage"
(https://github.com/bitcoin/bitcoin/pull/32516)
🤔 dergoegge reviewed a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2875707019)
Concept ACK

I'm not sure if the intermediate commits make things better here. Looking at the final diff seems simple enough.
💬 dergoegge commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2112292253)
```suggestion
Assume(peer->m_wtxid_relay == std::holds_alternative<Wtxid>(hash));
CInv inv(peer->m_wtxid_relay ? MSG_WTX : MSG_TX, hash.ToUint256());
```
💬 dergoegge commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2112303886)
not that it really matters but could do

```suggestion
std::optional<Txid> rejected_parent_reconsiderable;
```
🚀 fanquake merged a pull request: "log: Additional compact block logging"
(https://github.com/bitcoin/bitcoin/pull/32582)
💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2918910127)
Thanks for the reviews and the suggestions - added https://github.com/bitcoin/bitcoin/pull/32638 as a follow-up which enforces most remaining usages of `ReadBlock` to validate their hash.
💬 fanquake commented on pull request "windows: Use predefined `RC_INVOKED` macro instead of custom one":
(https://github.com/bitcoin/bitcoin/pull/32633#issuecomment-2918929592)
This needs rebasing for #32634; otherwise, I'm not sure how a release build of this was tested, given the Windows Guix build is broken in this branch.
💬 fanquake commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-2918940425)
https://cirrus-ci.com/task/6734127736553472?logs=ci#L3223
```bash
[12:12:21.305] /ci_container_base/src/test/reverselock_tests.cpp:23:9: error: cannot call function 'AssertLockNotHeldInline' while mutex 'mutex' is held [-Werror,-Wthread-safety-analysis]
[12:12:21.305] 23 | AssertLockNotHeld(mutex);
[12:12:21.305] | ^
[12:12:21.305] /ci_container_base/src/sync.h:141:31: note: expanded from macro 'AssertLockNotHeld'
[12:12:21.305] 141 | #define AssertLockNotHeld(cs
...
💬 hebasto commented on pull request "windows: Use predefined `RC_INVOKED` macro instead of custom one":
(https://github.com/bitcoin/bitcoin/pull/32633#issuecomment-2918956308)
> This needs rebasing for #32634; otherwise, I'm not sure how a release build of this was tested, given the Windows Guix build is broken in this branch.

Sure! Rebased.
💬 dergoegge commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-2918966148)
* Looking at `MaybeSetPeerAsAnnouncingHeaderAndIDs`, becoming a node's hb peer is pretty easy? I.e. be the first to supply the latest valid block at the tip and you'll be selected as high bandwidth (we process unsolicited block messages, so this should be trivial).
* Even if you're low-bandwidth can't you just send a headers announcement immediately followed by the compact block? As long as your headers message is the first announcement of the block, the compact block will be requested and sinc
...
🤔 stickies-v reviewed a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2877628308)
Concept ACK, the new `GenTxid` allows us to better benefit from the type safety of `Txid` and `Wtxid` and this PR takes a clear and structured approach that seems straightforward to review.

Approach-wise, I think I have a slightly different view. I find code generally easier to understand when we use the underlying types {`Txid`, `Wtxid`} directly and as such would prefer functions to be overloaded with those types, pushing conversion to the edge as much as possible. This also has the side be
...
💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2113608840)
In 811d2f34a0309a62a69897a15db47214bbfec776:

Isn't it more straightforward to just to have two `bool exists(const Txid& txid)` and `bool exists(const Wtxid& wtxid)` overloads?
💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2113548178)
nit: can also be implemented as a single three-way operator:

```suggestion
friend auto operator<=>(const GenTxid& a, const GenTxid& b) {
return std::tuple(a.index(), a.ToUint256()) <=> std::tuple(b.index(), b.ToUint256());
}
```

(needs to include `<compare>` and `<tuple>`)
💬 stickies-v commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2113659770)
Similarly to my comment about `TxMempool::exists()`, I think it would be better here to just overload `info`:

<details>
<summary>git diff on ad6d02720c</summary>

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index c0f6ac3f03..c34511d27d 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -5821,7 +5821,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
continue;
}

...