Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ w0xlt commented on pull request "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`":
(https://github.com/bitcoin/bitcoin/pull/32432#issuecomment-2864385970)
Thanks for the review and suggestion @musaHaruna . Added that text to the PR's description.
πŸ€” pablomartin4btc reviewed a pull request: "contrib: remove bdb exception from FORTIFY check"
(https://github.com/bitcoin/bitcoin/pull/32448#pullrequestreview-2826445260)
cr ACK f9dfe8d5e0d3f628659702ab781b7919505c829f

FWIW: Tried to run the check on Ubuntu and it seems some lief symbols have [changed](https://github.com/lief-project/LIEF/blob/main/doc/sphinx/changelog.rst) previously to the latest released version (< 0.16.5 - eg `lief.EXE_FORMATS.ELF` => `lief.ELF.Binary`), corrected it locally and ran succesfully, then saw that the CI has 0.13.2, this works well too.
πŸ’¬ murchandamus commented on pull request "coinselection: Optimize BnB exploration":
(https://github.com/bitcoin/bitcoin/pull/32150#issuecomment-2864494772)
The preceding PR https://github.com/bitcoin/bitcoin/pull/29532 got merged, so this is now ready for review.
πŸ‘ hodlinator approved a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2826464805)
ACK 477b69ad19d7fe37de9ca23926bcf007b309876d

### Concept

Paves the way for more intentional Windows manifest usage. [Linked documentation](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests) confirms that it can be used to control codepage and specify supported Windows versions.

### Approach

Not something I could have easily written from scratch but makes logical sense in how the new CMake function `add_windows_application_manifest` generates a manifest file
...
πŸ“ achow101 opened a pull request: "test: Remove legacy wallet RPC overloads"
(https://github.com/bitcoin/bitcoin/pull/32452)
`RPCOverloadWrapper` implemented overloads for legacy wallet only RPCs so that the same function call could be used within tests for both legacy wallets and descriptor wallets. With legacy wallets now removed, there is no need to continue to have these overloads.

For `importaddress`, `addmultisigaddress`, and `importpubkey`, the uses of these are converted to `importdescriptors`.

For `importprivkey`, a new helper function `wallet_imporprivkey` is introduced that does what the overload did.
...
πŸ“ JeremyRubin opened a pull request: "[Policy] Discourage Unsigned Annexes"
(https://github.com/bitcoin/bitcoin/pull/32453)
This patch adds a "redundant" rule with the annex discouragement, to discourage Annexes that are present but do not get signed.

This PR does not include tests, as the code paths are entirely redundant (and small!) with existing annex discouragement. If desired, tests could be made.

This new policy would come into play if someone somewhere for some reason decided that they were going to modify standard policy on their node to allow annexes **even though it is abundantly clear that annex pol
...
πŸ’¬ achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#discussion_r2080598493)
I could not figure out the CI failure, so I've reverted this change.
πŸ’¬ achow101 commented on pull request "wallet: Ensure best block matches wallet scan state":
(https://github.com/bitcoin/bitcoin/pull/30221#issuecomment-2864638615)
> Simplest fix would probably be to make the CWallet object a static variable in the function like the TestingSetup variable is, instead of a global variable.

It can't be static because the `FUZZ_TARGET` below needs to access it. However, the wallet could be created in there instead of in the setup, as the other wallet fuzz tests do.

But, with moving `WriteBestBlock` back to `RemoveWallet`, this is not necessary either.
πŸ‘ theStack approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2826561409)
Concept and code-review ACK 8c360d78b13fa7136db8d6e1e0af5d05f5141a64
πŸ’¬ theStack commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2080597980)
Seeing this constant also tricked me into thinking that the script size limit might play a role here. Agree that a comment would be nice, or maybe just bump it up to something much higher like e.g. 50000.
πŸ’¬ achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2864640606)
I think we could have a test for this in `wallet_backwards_compatibility.py`.
πŸ€” w0xlt reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2826580160)
ACK https://github.com/bitcoin/bitcoin/pull/32175/commits/61ea5f348da71b886807c0492587835dd7e57499

Good clarification.
πŸ’¬ rot13maxi commented on pull request "[Policy] Discourage Unsigned Annexes":
(https://github.com/bitcoin/bitcoin/pull/32453#issuecomment-2864661542)
Im surprised we haven’t seen any weird annex-use yet. This seems like a good safety policy to have in here until the annex’s use gets consensus. Could someone remove this? Sure, but at least theres sort of a warning sign that they might not want to.
πŸ’¬ davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2864794113)
I definitely opened this PR prematurely, thinking I could get away with just bumping the version and fixing the python scripts πŸ˜‚.

Working on getting guix builds working right, I referenced prior art by @willcl-ark here (https://github.com/willcl-ark/bitcoin/commit/ede9aa93fe6339fe6285c954dc8fbd9ae8623916).

The python build is convinced that a `build` option is set in `config-settings`, even when I override `configure-flags`, which is where `config-settings` is supposed to get it's value f
...
πŸ’¬ davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2080687125)
Yes, but it was to appease the CI linter, I understand now that this script is not meant for general use, so I changed this to just cast to `lief.Binary` without checking
πŸ’¬ 0xBEEFCAF3 commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2864936156)
>Let me see if there is anything that can be done to ensure it is never the case that OP_CAT = False && DISCOURAGE_OP_CAT = False since that should never happen outside of tests.

By default, the `SCRIPT_VERIFY_DISCOURAGE_OP_CAT` bit [is set](https://github.com/bitcoin/bitcoin/pull/29247/files#diff-1fc0f6b5081e8ed5dfa8bf230744ad08cc6f4c1147e98552f1f424b0492fe9bdR121), and we do not expose this as a configurable mempool policy. The only practical way for node operators to unset this bit (`SCRIP
...
πŸ“ jb55 opened a pull request: "tracing: fix invalid argument in mempool_monitor"
(https://github.com/bitcoin/bitcoin/pull/32454)
The mempool_monitor tracing tool is incorrectly reading the reason as the first argument, which is incorrect. Fix this!

Noticed this during the bitcoin++ mempool hackathon πŸ˜…

cc @0xB10C
πŸ’¬ Eunovo commented on pull request "Wallet: Fix Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in AddWalletDescriptor":
(https://github.com/bitcoin/bitcoin/pull/32344#issuecomment-2865239247)
> Including a range in a non-ranged descriptor import should cause the process to fail before reaching the wallet descriptor update function.

AFAICT this will cause `importdescriptor` to throw an error with the message "Range should not be specified for an un-ranged descriptor"
⚠️ hMsats opened an issue: "Self-compiled bitcoind 29.0 much slower than self-compiled 28.0 on my system"
(https://github.com/bitcoin/bitcoin/issues/32455)
I always compile the bitcoin software myself (`Ubuntu 24.04.2 LTS`). For `bitcoin-29.0` I compiled it (of course) for the first time with `cmake` (with and without `berkeley-db`).

For both (with and without `berkeley-db`), `bitcoind-29.0` is much slower than `bitcoin-28.0` which gives me problems on my server.

To investigate I wrote a shell script that determines the time difference in seconds between "Saw new header" and "UpdateTip":

```
2025-05-09T05:24:11Z Saw new header hash=000000000000
...