Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ€” furszy reviewed a pull request: "crypto: Use secure_allocator for `AES256_ctx`"
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-3443301033)
ACK 8bdcd12d3bcbeaf922fa10dc2a261848e3900cfd

No need to tackle the nano nits I left.
πŸ’¬ furszy commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2510608464)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a:
nano nit: I'm not sure how helpful this comment is.
πŸ’¬ furszy commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2510648818)
In https://github.com/bitcoin/bitcoin/commit/7176b26cde7cbaffdd92af9c25f85f8e5233e78a:
nano nit: you are already mentioning this above the for-loop line.
πŸ’¬ furszy commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2510613538)
In https://github.com/bitcoin/bitcoin/commit/7176b26cde7cbaffdd92af9c25f85f8e5233e78a:
Pedantic ultra-nano nit:
We finish comments with a dot only if they span more than one line.
πŸ’¬ hebasto commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511843037)
> > [Here](https://github.com/hebasto/bitcoin/commit/11bc5cca61a9edad38d070bd5046355fcf58c7ee#diff-4d05cd02fdce641be603f0f9abcecfeaf76944285d4539ba4bbc40337fa9bbc2) is IWYU's diff based on #33810:
>
> I can't find that in the CI output. I don't think iwyu runs on stand-alone headers without a cpp file?

It's here: https://github.com/hebasto/bitcoin/actions/runs/19194191546/job/54872770781:
```
<snip>
2025-11-08T14:21:35.1873049Z (/home/runner/work/_temp/src/kernel/bitcoinkernel_wrapper.h
...
πŸš€ fanquake merged a pull request: "Update `minisketch` subtree"
(https://github.com/bitcoin/bitcoin/pull/32856)
πŸ’¬ Crypt-iQ commented on pull request "fuzz: compact block harness":
(https://github.com/bitcoin/bitcoin/pull/33300#discussion_r2510713428)
Ran this patch against master with -reindex (which puts lots of entries in `vBlocks`) and noticed no slowdown.

cc @l0rinc, do you have any opinions about this? We need this for deterministic fuzzing, we can also wrap this in a fuzz-specific macro.
⚠️ charletonjoan1 opened an issue: "REPORT AND RECOVER MONEY BACK FROM A FRAUDULENT CHARITY/DONATION SCAM.HIRE META TECH RECOVERY PRO"
(https://github.com/bitcoin/bitcoin/issues/33839)
A charity organization popped up on my screen as an ad. They had a professional-looking website, emotional stories, and testimonials from supposed donors. They had pictures of their representatives with A-list actors, celebrities, and philanthropists as donors; unfortunately, they were edited. I read about their services and outreach, and I was motivated to give a token as a widow with no children or close family relatives. They claimed the funds sourced were being used to support families in n
...
πŸ’¬ maflcko commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2510727269)
> k, I had the impression this was the only one πŸ‘

Oh, I think I misunderstood the previous comments. Clearly a test-only workaround to a single file is better than a ci-wide workaround to all files.

The test code is "wrong" anyway. Using a pointer here makes little sense, when a reference should be used.
πŸ‘ TheCharlatan approved a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443473061)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9
πŸ’¬ fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511924176)
> Even if it’s been fixed upstream,

It's not clear to me if it has been fixed or not, can you link to the relevant change / issue? I can cherry-pick the change above in, if you create a commit, and add a comment inline, for when the change can be dropped.
πŸ“ maflcko opened a pull request: " test: [refactor] Use reference over ptr to chainman "
(https://github.com/bitcoin/bitcoin/pull/33840)
Just some minor test-only refactor commits to fix GCC false positive warnings, along with making the test code easier to read and understand:

* First change requested in https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2510727269
* Second change requested in commit 3b135a8fc4451c93b3ea50b3f4621e0d19f35daf

Those changes are required in a bunch of pulls touching the CI system, so merging them allows to drop them in all pulls.
πŸ“ hebasto opened a pull request: "cmake: Switch to `minisketch` upstream build system"
(https://github.com/bitcoin/bitcoin/pull/33841)
This PR:
1. Is split out from https://github.com/bitcoin/bitcoin/pull/32856 as [requested](https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3511268570).

2. Switches to `minisketch`'s own build system script.

3. Does not change behaviour, except for printing "minisketch configure summary" in the build configuration output.

Fellow reviewers might also be interested in the discussion that took place in https://github.com/bitcoin/bitcoin/pull/32856.
πŸ’¬ Crypt-iQ commented on issue "ASAN use-after-free in `m_reconnections`":
(https://github.com/bitcoin/bitcoin/issues/33615#issuecomment-3512126349)
@big14way I was planning on fixing this issue myself, but if you want to fix it, feel free. I think it'd be valuable if you were able to reproduce the use-after-free to make sure that the issue is fixed.
πŸ’¬ optout21 commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#issuecomment-3512132153)
Concept ACK

I can't judge the probability of mutability check here needed again in the future, but in that case, it can be added.
πŸš€ fanquake merged a pull request: "test: rpc_bind: Skip nonloopback test if no such address is found"
(https://github.com/bitcoin/bitcoin/pull/33433)
βœ… fanquake closed a pull request: "Add option dbfilesize to control LevelDB target ("max") file size"
(https://github.com/bitcoin/bitcoin/pull/30059)
πŸ’¬ fanquake commented on pull request "Add option dbfilesize to control LevelDB target ("max") file size":
(https://github.com/bitcoin/bitcoin/pull/30059#issuecomment-3512199526)
Closing for now.
πŸ’¬ sipa commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2510907674)
@ajtowns I don't think a virtual move assignment would actually help. It's true that there are risks when performing (move) assignments across types, but it doesn't seem like making it virtual affects those much.

With the code, as it exists in this PR, the risk is something like:
```
TxGraph::Ref ref = ...;
TxMempoolEntry entry = ...;
...
ref = std::move(entry);
// or
entry = std::move(ref);
```

The first of which will "excise" the ref from the entry, leaving the entry ref-less. Th
...
πŸ“ maflcko opened a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842)
All supported operating systems that previously came with at least g++-11, also come with at least g++-12, so bumping the minimum should be fine.