Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👋 waketraindev's pull request is ready for review: "rpc, test: fix "internal" label check in importdescriptors and extend test"
(https://github.com/bitcoin/bitcoin/pull/33614)
⚠️ Crypt-iQ opened an issue: "ASAN use-after-free in `m_reconnections`"
(https://github.com/bitcoin/bitcoin/issues/33615)
I ran into this while shutting my node down during IBD. I think it's benign.

What happens:
- `CConnman` is started [here](https://github.com/bitcoin/bitcoin/blob/64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa/src/init.cpp#L2183).
- This initializes `semOutbound` [here](https://github.com/bitcoin/bitcoin/blob/64a7c7cbb975cb3c3f25a3f784779f32cd95ebaa/src/net.cpp#L3361).
- While `CConnman` is active, `m_reconnections` is added to in `DisconnectNodes` [here](https://github.com/bitcoin/bitcoin/blob/64a7
...
📝 instagibbs opened a pull request: "2025 10 bypass checkephemeral"
(https://github.com/bitcoin/bitcoin/pull/33616)
Similar reasoning to https://github.com/bitcoin/bitcoin/pull/33504

During a deeper reorg it's possible that a long sequence of dust-having transactions that are connected in a linear fashion, and currently each subsequent "generation" will be rejected on reorg, even though the dust would be spent usually by another child transaction. This could evict a large amount of competitive fees via normal means.

PreCheck based `PreCheckEphemeralSpends` is left in place because we wouldn't have relay
...
💬 fjahr commented on pull request "refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#issuecomment-3398490065)
Turning it off and on again finally worked :p
💬 janb84 commented on pull request "build: Bump clang minimum supported version to 17":
(https://github.com/bitcoin/bitcoin/pull/33555#issuecomment-3398591114)
ConceptACK fa0fa0f70087d08fe5a54832b96799bd14293279

PR Sets the clang minimum supported version to 17 and drop some work arounds for clang 16. Given that supported OS-es have already a minimal clang version of 17, this should not be a problem.

Would like to see confirmation that the fuzz test works as expected, by a fuzz specialist.

- code review
- GUIX builds
- normal build
- ran tests

Ps. NixOS 25.05 has Clang version 19 as default.

<details><summary> Guix Build O
...
👍 stickies-v approved a pull request: "refactor: Construct g_verify_flag_names on first use"
(https://github.com/bitcoin/bitcoin/pull/33600#pullrequestreview-3332643263)
ACK faa9d10c84bc6b465cbca266468990cc716b4300

I would prefer the `constexpr` approach, but either approach is acceptable to me.
💬 stickies-v commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#discussion_r2427048878)
An alternative approach would be to use a `constexpr` array and make the whole thing compile-time? As a nice benefit, removes the `FLAG_NAME` macro which imo doesn't simplify things enough to be worth it.

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

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index abd99fc365..731c0a070f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(con
...
💬 janb84 commented on pull request "rpc, test: fix "internal" label check in importdescriptors and extend test":
(https://github.com/bitcoin/bitcoin/pull/33614#discussion_r2427075461)
NIT: would rather see `internal == true` imho better readability.

```suggestion
if (internal == true && data.exists("label")) {
```
💬 janb84 commented on pull request "rpc, test: fix "internal" label check in importdescriptors and extend test":
(https://github.com/bitcoin/bitcoin/pull/33614#discussion_r2427079894)
NIT: restore this and use the pattern that is used for the rest of the tests. Change the `import_request` slightly e.g:

``` C
self.log.info("Test can import a p2pkh descriptor with internal explicit set to false ")
self.test_importdesc({**import_request, "internal": False}, success=True)
```
💬 waketraindev commented on pull request "rpc, test: fix "internal" label check in importdescriptors and extend test":
(https://github.com/bitcoin/bitcoin/pull/33614#issuecomment-3398759053)
Resolved nits, internal checks for true, test updated.
💬 sipa commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3399048111)
Imagine the following state:

```mermaid
graph BT

g["genesis"];
f["fork_point"];
a["ActiveTip()"];
b["pindexBestKnownBlock"];
l["pindexLastCommonBlock"];
p["P"];
f-->g;
a-->f;
l-->p;
b-->p;
p-->f;
```

In this situation, I think we want to update `pindexLastCommonBlock` to be `P`, but the current code will select `fork_point`. Is this a situation that can occur?

I think that the logic we actually want is selecting the more-work among `LastCommonAncestor(pindexBestKnownBlock
...
💬 ajtowns commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#discussion_r2427318037)
I had a look at this approach too. With C++20, I believe you can ensure the array is sorted at compile time with:

```c++
template<typename V, size_t N>
consteval auto SortedArray(const std::array<V,N>& pairs)
{
std::array<V,N> result = pairs;
std::sort(result.begin(), result.end());
return result;
}
#define FLAG_NAME(flag) std::pair<std::string_view, script_verify_flag_name>{#flag, SCRIPT_VERIFY_##flag}
constexpr auto g_verify_flag_names = SortedArray(std::array{
FLA
...
💬 ajtowns commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#discussion_r2423373072)
`static const auto& mFN = SFNTE();` ?
💬 ajtowns commented on pull request "refactor: Construct g_verify_flag_names on first use":
(https://github.com/bitcoin/bitcoin/pull/33600#discussion_r2427284052)
The value of the `FLAG_NAME` macro is that it makes any typos in the strings that would cause a mismatch with the enum names an immediate compile time error. It also makes it easier to type and easier to review. Massive -1 on dropping it. C++26 reflection would be better, when it's available, of course.
🤔 pablomartin4btc reviewed a pull request: "leveldb: show non-default options during init"
(https://github.com/bitcoin/bitcoin/pull/31644#pullrequestreview-3333105034)
Concept ACK

Perhaps the legend "with options: ..." could be debug log? Ideally in the same line but I don't think we support it actually. As an alternative (haven't tested it) maybe:

```
const bool log_options = LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug);

LogInfo("Opened LevelDB successfully%s",
log_options
? tfm::format(" with options: %s", GetChangedOptions(DBContext()))
: "");
```
📝 ac12644 opened a pull request: "contrib: Fix gen-manpages.py to check build options"
(https://github.com/bitcoin/bitcoin/pull/33617)
The gen-manpages.py script was missing critical build option checks,
causing incomplete man page generation. This commit adds comprehensive
build option detection to ensure all available options are documented.

Changes made:
- Add missing USE_UPNP build option detection (core requirement from #17506)
- Fix grammar error: "Aborting generating..." → "Aborting generation of..."
- Remove unused build options that were listed but not actually used
- Add error handling for missing 'components
...
🤔 sipa reviewed a pull request: "miner: fix empty mempool case for waitNext()"
(https://github.com/bitcoin/bitcoin/pull/33566#pullrequestreview-3333348405)
utACK 8f7673257a1a86717c1d83770dc857fc254df107
⚠️ Christewart opened an issue: "`generatetoaddress` 2-3x slower on v30 compared to v29"
(https://github.com/bitcoin/bitcoin/issues/33618)
Hardware

```
Darwin Chriss-MacBook-Pro.local 24.6.0 Darwin Kernel Version 24.6.0: Mon Jul 14 11:30:55 PDT 2025; root:xnu-11417.140.69~1/RELEASE_ARM64_T6031 arm64
```


### v29

```
Bitcoin Core daemon version v29.0.0
Copyright (C) 2009-2025 The Bitcoin Core developers
```

```
$ time bitcoin-cli -regtest generatetoaddress 2000 $(bitcoin-cli -regtest getnewaddress)
0.01s user 0.01s system 0% cpu 1:56.41 total
```

### v30

```
Bitcoin Core daemon version v30.0.0
Copyright (C) 2009-2025 The B
...
💬 mzumsande commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3399660829)
I tried the same on Ubuntu / SSD and didn't see a difference.
Just to make sure: Did you start with an empty datadir and fresh wallet each run?
💬 l0rinc commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3399937858)
I have also measured it a few different ways:

Compiling from source and running it 3 times for `v29.2` vs `v30.30`:
<details>
<summary>generatetoaddress | i9-ssd | x86_64 | Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz | 16 cores | 62Gi RAM</summary>

```bash
VERSIONS="29.2 30.0"; \
BASE_DIR="/mnt/my_storage"; DATA_DIR="$BASE_DIR/BitcoinRegtestData"; LOG_DIR="$BASE_DIR/logs"; \
git fetch origin --tags && \
for v in $VERSIONS; do \
echo "Building v$v..."; \
git checkout v$v && \
rm -rf build-$v
...