Bitcoin Core Github
44 subscribers
119K links
Download Telegram
⚠️ mzumsande opened an issue: "net: multiple-node local setups with different `-port` don't work anymore"
(https://github.com/bitcoin/bitcoin/issues/31134)
With v28.0, (PR #22729), we now abort init on failed binds, as documented in the release notes. This also affects the default onion bind to port `8334`, which is always done unless the user specifies the desired binds using the `-bind` command.

As a result, setups with two local nodes that each use a different `-port` are not possible anymore - both of these nodes would attempt to create an onion bind to port `8334`, and one of them would fail. In earlier versions, we would silently ignore th
...
💬 fanquake commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2429530060)
cc @stevenroose
💬 mzumsande commented on issue "net: multiple-node local setups with different `-port` don't work anymore":
(https://github.com/bitcoin/bitcoin/issues/31134#issuecomment-2429533052)
closed, #31133 was opened at the same time, let's discuss there.
mzumsande closed an issue: "net: multiple-node local setups with different `-port` don't work anymore"
(https://github.com/bitcoin/bitcoin/issues/31134)
💬 Christewart commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#discussion_r1810921124)
Is there perf reasons to put the `auto result2 = check();` into the `if` expression? Seems more readable to do

```c++
auto result2 = check2();
if (!result2.has_value()) {
...
}
```

IIUC I see why this is done on [`L2187`](https://github.com/bitcoin/bitcoin/pull/31112/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R2187) - to avoid doing the computation unnecessarily.
📝 fanquake converted_to_draft a pull request: "[28.x] Some backports"
(https://github.com/bitcoin/bitcoin/pull/31104)
Backports:
* #31007
* #31016
* #31035
💬 andrewtoth commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#issuecomment-2429557412)
Not sure this approach is worth it anymore. For a full IBD I got about 2% faster. It seems that keeping non-dirty entries in the cache to be read from if spent is not that much benefit, since the likelihood of one of those entries being spent before evicted is not high enough.

I think #31132 is a better approach, because cache misses are fetched in parallel much faster so the misses are not as important.

I still think #28939 should be revived though. It is important to also track the memor
...
andrewtoth closed a pull request: "Don't wipe coins cache when full and instead evict LRU clean entries"
(https://github.com/bitcoin/bitcoin/pull/31102)
📝 jonatack opened a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31135)
in getblockchaininfo/-getinfo and getchainstates, as requested in issues https://github.com/bitcoin/bitcoin/issues/31127 and https://github.com/bitcoin/bitcoin/issues/26433. Verification progress estimates in the debug logging remain unchanged.
💬 jonatack commented on issue "getblockchaininfo `verificationprogress` never reaches 1.0":
(https://github.com/bitcoin/bitcoin/issues/31127#issuecomment-2429580257)
> Maybe for UX purposes `verificationprogress` in RPCs getblockchaininfo and getchainstates (and CLI -getinfo) could return 1 instead of the estimate, when `blocks` equals `headers` or `validated` is true.

Proposed in https://github.com/bitcoin/bitcoin/pull/31135.
💬 real-or-random commented on pull request "cleanse: switch to SecureZeroMemory for Windows cross-compile":
(https://github.com/bitcoin/bitcoin/pull/26950#issuecomment-2429613882)
@fanquake Can you elaborate on the motivation for this PR? The code in the `#else` should work for any gcc and clang, on any platform. While it uses an asm block, there's no actual asm instruction inside that could be platform-specific.

See also its platform-independent usage in Linux:
https://github.com/torvalds/linux/blob/c2ee9f594da826bea183ed14f2cc029c719bf4da/include/linux/string.h#L369-L376
https://github.com/torvalds/linux/blob/c2ee9f594da826bea183ed14f2cc029c719bf4da/include/linux
...
💬 fanquake commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2429624992)
Note that this issue isn't Nix specifix, the coverage build type is somewhat broken (see also #31047). The steps to reproduce result in the same/similar failure on non-nix platforms.
🤔 theuni reviewed a pull request: "depends: add *FLAGS to gen_id"
(https://github.com/bitcoin/bitcoin/pull/31125#pullrequestreview-2385624483)
Concept ACK
💬 theuni commented on pull request "depends: add *FLAGS to gen_id":
(https://github.com/bitcoin/bitcoin/pull/31125#discussion_r1810980328)
So why bother passing them? I don't see any need to rebuild the native packages when changing host flags?
⚠️ TheBlueMatt opened an issue: "Expose `TestBlockValidity` over RPC"
(https://github.com/bitcoin/bitcoin/issues/31136)
### Please describe the feature you'd like to see added.

Not entirely sure it tests all types of block validity, but it looks like it nearly does? If it does, its generally quite useful to have a way to as Bitcoin Core if a block would be valid, ignoring its PoW, so having that be public would be nice.

### Is your feature related to a problem, if so please describe it.

_No response_

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No resp
...
👍 hebasto approved a pull request: "build: Fix kernel static lib component install"
(https://github.com/bitcoin/bitcoin/pull/31078#pullrequestreview-2385670659)
ACK 82e16e698321ea6fb69ce5a08b048347aab8c74e, tested on Ubuntu 23.10.
💬 fanquake commented on issue "RPC breakage with v28.0":
(https://github.com/bitcoin/bitcoin/issues/31039#issuecomment-2429732718)
Going to leave this open a little longer to see if anything else comes up that might warrant a backport or other action.
💬 laanwj commented on issue "ci: Replace wine tests with real tests on Windows?":
(https://github.com/bitcoin/bitcoin/issues/31071#issuecomment-2429744574)
Concept ACK. WINE tests are better than nothing, but clearly running the tests on real windows is better. It is not our job to test WINE. They were introduced because there were no usable CI systems for windows at the time.
💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1811061628)
This only exists for sanity checking that we don't have more than one changeset at a time. We could just as easily replace this with a `bool`; I made it a pointer mostly for debugging purposes, if the `Assume()` were ever hit.

The idea behind the `unique_ptr` is that I wanted all the state in the ChangeSet to magically go away whenever the object goes out of scope, so that I didn't have to explicitly clean up state everywhere that a transaction might fail validation (which seems like a lot o
...
💬 mzumsande commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2429776658)
I do think that keeping `-port` around could be helpful, especially for users less knowledgeable about networking, who aren't sure which address to put in an equivalent `-bind` command.

I also like option 2, provided that the behavior will be well-documented, so that users specifying `-port` values for multiple nodes don't pick consecutive ones by accident (which would then again lead to collisions). Otherwise I don't see a downside.

I would also be ok with option 6, if I specify a port li
...