Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2435278993)
The main issue this PR addresses is that currently, no man pages are generated if a single binary is missing. The goal is to skip the missing binary and generate man pages for the others, as was done in the previous shell script.

If I understand correctly, you want to ensure no information is lost in the man pages, right? In that case, I can simply remove the following line:
```py
42 BINARIES.remove(relpath)
```
This way, the existing binaries will have their man pages generated,
...
👍 stickies-v approved a pull request: "Package validation: accept packages of size 1"
(https://github.com/bitcoin/bitcoin/pull/31096#pullrequestreview-2385195309)
ACK 303661871debadee5f67bd7e4cd0cccc85344ef2

I think reducing the special (and imo unexpected) treatment of packages of transactions without unconfirmed parents by this PR is an improvement. I looked at the call hierarchy of `IsChildWithParents()`, and the changes look safe to me, but I am not super familiar with this part of the code.
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1810721390)
Orthogonal to the changes here, but related and I think it would be good to add this assert + doc to `IsChildWithParentsTree()` too, if you force-push.

<details>
<summary>git diff on 92ed1bd586</summary>

```diff
diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp
index 38f26a9c4b..e34f94cbb1 100644
--- a/src/policy/packages.cpp
+++ b/src/policy/packages.cpp
@@ -138,6 +138,7 @@ bool IsChildWithParentsTree(const Package& package)
{
if (!IsChildWithParents(package)) r
...
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1810732642)
I think I agree with instagibbs that adding complexity on top should be avoided if we can, and it seems to me like modifying `IsChildWithParents()` to better align with the `Package` definition (i.e. child and all unconfirmed parents, as per `AcceptPackage()`) is preferable for long-term robustness, over special-casing a transaction that has 0 unconfirmed parents as is currently the case.

I looked at the callstacks that make use of `IsChildWithParents()`, and since the scope is fairly limited
...
💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2435308803)
Kind of. The problem is that the command will still finish successfully in case of an incomplete build (or build configuration), while the before a release it's critical to update all the pages.

This is why i suggested to add this as an option eg `--skip-missing-binaries`.

> This way, the existing binaries will have their man pages generated, and all the other man pages will still be referenced at the bottom, even if the binaries are missing.

That is an improvement, but doesn't entirely
...
💬 Sjors commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2435316425)
cc @Emzy
💬 1440000bytes commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2435336204)
> Miniupnpc is a C library (somewhat) maintained by a single person which had several vulnerabilities in the past (a couple dozens are listed here), some of which directly affected our software (RCE in 2015, OOM in 2020).

and CVE-2017-1000494 in 2018?

Concept ACK

Lint error: https://github.com/bitcoin/bitcoin/pull/31130/checks?check_run_id=31973806317
💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2435337573)
`/Users/sjors/dev` is a symlink to `/Volumes/SSD/Dev/`. I'll try a build without using a symlink.
💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2435345680)
Looks like switching from Assume to assert in BitSet has a noticeable impact on the `Linearize*` bench tests: https://corecheck.dev/bitcoin/bitcoin/pulls/31093
💬 laanwj commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1815036443)
maybe
"If you want to open a port automatically, consider using the `-natpmp` option instead, which uses PCP or NAT-PMP depending on router support."
(in the GUI, the option is still called NATPMP as well, so no need to mention that seperately i think)
💬 1440000bytes commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2435361835)
Is it possible for core to remove support for testnet to avoid maintenance burden?

Regtest and Signet could work fine for testing. Maybe testnet could be supported by some fork if there is enough demand.
👍 TheCharlatan approved a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147#pullrequestreview-2392642727)
ACK fb46d57d4e7263495c007f4a499a349bff2b21e0
fanquake closed an issue: "How to compile the GUI on opensuse tumbleweed with cmake?"
(https://github.com/bitcoin-core/gui/issues/842)
🚀 fanquake merged a pull request: "cmake, qt, test: Remove problematic code"
(https://github.com/bitcoin/bitcoin/pull/31147)
📝 furszy opened a pull request: "ci: display logs of failed unit tests automatically"
(https://github.com/bitcoin/bitcoin/pull/31148)
Saw it here https://github.com/bitcoin/bitcoin/actions/runs/11488618084/job/31975712362?pr=31000.

The 'test-each-commit' and 'win64' CI jobs currently do not display test logs when an error occurs, making it almost impossible to debug issues that don't happen locally. Fix this by setting the CTest `--output-on-failure` flag (per [README](https://github.com/bitcoin/bitcoin/blob/2f40e453ccdf5f75b599fc7168abd512510686c5/src/test/README.md?plain=1#L130)).
💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815047765)
Done.
💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815048051)
Done. The whole dump is already compared a few lines below, but I guess it can't hurt to the detailed address->label entry checks in addition.
💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1815048142)
Done.
💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-2435378650)
Thanks for the review! Rebased on master (as I suspect CI would be very unhappy otherwise) and addressed comments https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813809067, https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813825732 and https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1813830674
👋 l0rinc's pull request is ready for review: "optimization: pack util::Xor into 64/32 bit chunks instead of doing it byte-by-byte"
(https://github.com/bitcoin/bitcoin/pull/31144)