Bitcoin Core Github
43 subscribers
123K links
Download Telegram
👍 TheCharlatan approved a pull request: "depends: remove `PYTHONPATH` from config.site"
(https://github.com/bitcoin/bitcoin/pull/28845#pullrequestreview-1725750947)
ACK 95aab1f02784e5d92572ccfcd4ce27107e4b2888

There is nothing in that path anymore.

Guix builds (x86_64)
```
92c922498b9d7e68742355653cc84317d0f0947e8fdd012488c9875e8b54e03c guix-build-95aab1f02784/output/aarch64-linux-gnu/SHA256SUMS.part
634a6a7a82efcb86eb7e875aa4534bd5646e80ee21e951d9624ca7ff78fc82f5 guix-build-95aab1f02784/output/aarch64-linux-gnu/bitcoin-95aab1f02784-aarch64-linux-gnu-debug.tar.gz
2dd4305e325723cac4b0b8e1803d48f3d7eb6fa2fd49c0a980ff907bb3e5da2b guix-build-95aab1
...
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1389995261)
Because `tx3` replaced `tx2`, so `tx1` is now `TxStateInactive` instead of `TxStateMempoolConflicted`, making `unspents[0]` spent.
⚠️ luke-jr opened an issue: "Manpage for `-par` bolds lower bound"
(https://github.com/bitcoin/bitcoin/issues/28850)
```
\fB\-par=\fR<n>
.IP
Set the number of script verification threads (\fB\-10\fR to 15, 0 = auto, <0 =
leave that many cores free, default: 0)
```

`\fB` makes the `-10` bold, as if it were a parameter rather than a value.

(Additionally, the `-10` comes from the system of whoever ran the gen-manpages script, and should probably be generated deterministically here...)
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1390030476)
Hmm, that will only happen when the node is close to the tip, because of the 1024 block download window. More precisely, when the node is within 1312 blocks from the tip (window + limited peers threshold). Nice observation!.
Will modify it and add coverage for it.
📝 hebasto opened a pull request: "build: Patch Qt to handle minimum macOS version properly"
(https://github.com/bitcoin/bitcoin/pull/28851)
This PR is:
- required to [switch](https://github.com/bitcoin/bitcoin/pull/28622) to macOS 14 SDK (Xcode 15).
- an alternative to https://github.com/bitcoin/bitcoin/pull/28732 and https://github.com/bitcoin/bitcoin/pull/28775.

Qt relies on the `__MAC_OS_X_VERSION_MIN_REQUIRED` macro, which is set in the `AvailabilityInternal.h` SDK header to
the value provided by the Clang driver from the `-mmacos-version-min` / `-mmacosx-version-min` option.

Xcode 12 SDK expects the OS-specific `__ENVI
...
💬 hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1806576609)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/28851.
hebasto closed a pull request: "build: Patch Qt to handle minimum macOS version properly"
(https://github.com/bitcoin/bitcoin/pull/28775)
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390055024)
These two things are not technically equivalent, because `send` creates a change output here, which results in intermittent failures for the rest of the test. Is there any way to tell `send` not to create a change output?
💬 ajtowns commented on pull request "[refactor] Remove BlockAssembler m_mempool member":
(https://github.com/bitcoin/bitcoin/pull/28843#discussion_r1390058040)
Why aren't you just changing `addPackageTxs` to use `m_mempool` directly? Seems simpler, and doesn't change the public api. Something like https://github.com/ajtowns/bitcoin/commit/944d8bc87690389de9bd51a6ea8a3223d10e4002#diff-352b412a72c7d069b77b786164c6681ee324ff0d8e61f01d48057e297db991fa
💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390071466)
I think that `TxStateBlockConflicted` and `TxStateMempoolConflicted` are a bit more clear, because with something like `TxStateUnconfirmedConflict` instead of `TxStateMempoolConflicted`, it is unclear whether the conflict is in the mempool, or just hasn't been broadcasted yet. Additionally, I felt that it was more uniform with the other transaction states to be describing the state of the transaction instead of the state of a conflict. Eg. `TxStateBlockConflicted` indicates that the transaction
...
🤔 furszy reviewed a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1725862532)
Updated per feedback, thanks for the review @vasild!

The node will now request blocks within the limited peers threshold when the block download window contemplates them and not only when the previous blocks arrived or are in-flight. Have added test coverage exercising this behavior and verifying that the node can get synced even when the blocks arrived out of order.
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1390079370)
Removed the unconditional wait on the last update :).
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1390075262)
Ended up doing it in a slightly different way. Using `setnetworkactive` to isolate the full node from the other two. I think that is clearer in this way.
💬 furszy commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1390079461)
Added `NODE_NETWORK_LIMITED_MIN_BLOCKS` on the last update.
💬 Abuchtela commented on pull request "Add support for "partial" fuzzers that indicate usefulness":
(https://github.com/bitcoin/bitcoin/pull/27552#issuecomment-1806630463)
`Rebase`
💬 theStack commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1390101144)
One small drawback of the current way of inspecting `extra_args` for the v2transport option is that it doesn't work if the argument is specified in a slightly different format with same meaning. For example, `-v2transport=0`/`-v2transport=1` could also be passed as `-nov2transport`/`-v2transport`, `-nov2transport=1`/`-nov2transport=0` (admittedly, no sane person would use the last one though). It might be worth it to consider that in a follow-up, e.g. by enforcing with an assertion that argument
...
👍 theStack approved a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1725906389)
ACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe
💬 stratospher commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1390105880)
> "if no extra_args are passed, restart with the ones that were initially specified (self.extra_args)"

oh interesting! so https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1388909333 is out of scope for this PR.
🤔 stratospher reviewed a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1725912169)
ACK 35fb993.
💬 sipa commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1806662743)
utACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe. Thanks for taking care of this.