Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-3101714725)
This needed a release note :-)

Maybe we can still mention it in the next v29 update?
💬 fanquake commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-3101723352)
https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-29.0.md#build-system

> By default, the built executables and libraries are located in the bin/ and lib/ subdirectories of the build directory.
📝 maflcko opened a pull request: "doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)"
(https://github.com/bitcoin/bitcoin/pull/33037)
ref https://github.com/bitcoin/bitcoin/pull/32521
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2221785394)
`ImportBlocks` first tries to load blocks from existing block files during the reindex process. After the reindex process, it tries to load any external blocks provided by the user. We only want to force the use of assumevalid(if set) during the reindex process, not when loading external blocks.

We call `activate_all_chainstate` the first time, during the reindex process. This allows us to skip script checks using modified assumevalid rules for reindex.
We call `activate_all_chainstate` the
...
💬 maflcko commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3101732454)
Created https://github.com/bitcoin/bitcoin/pull/33037 and removed the 'needs release notes' label.
💬 maflcko commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3101773916)
> It can track which clients and versions support them. And list which ones are configurable in clients. And finally which clients have different defaults.

You can use `$ ./bld-cmake/bin/bitcoin-qt -? | grep 'Node relay options:'` to get the list of configurable standardness rules and their default values for all versions you are interested in. There is also https://github.com/bitcoin/bitcoin/tree/master/doc/policy, but it isn't complete. I am not sure if it makes sense to try to list each th
...
🤔 maflcko reviewed a pull request: "net: make m_mempool optional in PeerManager"
(https://github.com/bitcoin/bitcoin/pull/33029#pullrequestreview-3042100735)
I think the changes are conceptually nice, but hard to see a user-visible benefit from this alone. Combined with the difficulty to review this (in its current state) around nullptr deref, it will probably be a hard sell.
💬 maflcko commented on pull request "net: make m_mempool optional in PeerManager":
(https://github.com/bitcoin/bitcoin/pull/33029#discussion_r2221894883)
I don't understand why you removed the assert (https://github.com/bitcoin/bitcoin/pull/26247/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1161)

`PartiallyDownloadedBlock` can't deal with nullptr and dereferencing a nullptr will be UB and will usually lead to a crash.
💬 maflcko commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3101889213)
> all good now

no, it is not. This can be seen if you take a look at the changes yourself.

Also, it would have to be squashed, as already explained above as well.

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 maflcko commented on pull request "test: Add functional tests for blockreconstructionextratxn parameter":
(https://github.com/bitcoin/bitcoin/pull/33023#issuecomment-3101914384)
Could turn into draft while CI is failing?
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2221936637)
> However, the descriptor unit tests often call ToPrivateString() on watch-only descriptors

Yeah, I don't know why this was done.

> To add this assertion, we will have to remove those checks, and I haven't seen the advantage in doing that.

Agree, doesn't need to be done right now.
💬 maflcko commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2221958115)
less relevant here, but below you are removing the assert on the `CInv&`. Seems fine to at least restore this as an Assume?
⚠️ fanquake opened an issue: "SegFault in QSortFilterProxyModelPrivate::build_source_to_proxy_mapping"
(https://github.com/bitcoin-core/gui/issues/880)
Moved from https://github.com/bitcoin/bitcoin/issues/32957.

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

I was loading a bunch of old wallets sequentially using "bitcoin-cli loadwallet" in a loop. bitcoin-qt crashed shortly after finishing loading one wallet and starting to load the next.

The debug log shows this at the end:

2025-07-13T13:06:26Z init message: Rescanning…
2025-07-13T13:06:26Z [wallet23] Rescanning last 3130
...
💬 fanquake commented on issue "SegFault in QSortFilterProxyModelPrivate::build_source_to_proxy_mapping":
(https://github.com/bitcoin-core/gui/issues/880#issuecomment-3101966993)
I just had the same crash happen again. This time very shortly after starting bitcoin-qt, while it was still loading the mempool from disk. I ran the 'loadwallet' loop again, and it crashed on the 2nd wallet in the loop. There's one wallet loaded from `bitcoin.conf` where it says "wallet=wallet1". I waited for that to finish loading before starting the "bitcoin-cli loadwallet $w" loop.

Here's the debug.log:

2025-07-13T22:25:10Z Leaving InitialBlockDownload (latching to false)
2025-07-1
...
fanquake closed an issue: "SegFault in QSortFilterProxyModelPrivate::build_source_to_proxy_mapping"
(https://github.com/bitcoin/bitcoin/issues/32957)
💬 fanquake commented on issue "SegFault in QSortFilterProxyModelPrivate::build_source_to_proxy_mapping":
(https://github.com/bitcoin/bitcoin/issues/32957#issuecomment-3101967885)
Moved to https://github.com/bitcoin-core/gui/issues/880.
💬 fanquake commented on issue "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files":
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-3101971822)
PR open in the maintainer tools repo: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/185.
💬 Sjors commented on pull request "doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)":
(https://github.com/bitcoin/bitcoin/pull/33037#issuecomment-3101974508)
Maybe mention that these rules match the proposed consensus changes in BIP54.
💬 maflcko commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2222082548)
Could also mention the RPC field removals?

Also, if you want to fix typos, the LLM found a few:

tranaction -> transaction [misspelling in comment “memory usage of the tranaction”, and "latency score for all tranactions"]
comphehensive -> comprehensive [misspelling in comment “This is a comphehensive simulation…”]
Remove trailing “)” in “We must now be within limits, otherwise LimitOrphans should have continued further).” [unmatched parenthesis]

But up to you.