Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ€” hebasto reviewed a pull request: "build: add `-W*-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482#pullrequestreview-3442877665)
Tested 2e713c22c67b3551f04273428db266503178e7fa on Ubuntu 25.10 with GCC 15.2.0:
```
$ cmake --build build
<snip>
[340/1092] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp.o
src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing whitespace [-Wtrailing-whitespace=]
5603 |
src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:8254:2: warning: trailing whitespace [-Wtrailing-whitespace=]
8254 |
src/qt/bitcoinqt_autogen/EWIEG
...
πŸ’¬ fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511243308)
The minisketch issues are solved with pulling the subtree. I guess we are now just blocked on Qt / GUI tooling...
πŸ’¬ fanquake commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3511268570)
Any chance you want to turn this into just a subtree pull, and do the switchover in a followup PR?
πŸ€” rkrux reviewed a pull request: "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`"
(https://github.com/bitcoin/bitcoin/pull/31734#pullrequestreview-3443008539)
Concept ACK 28a4fcb03c0fb1cd5112eca1eb36dcb13e0b4ff2

Thanks for adding the fix!

The PR description can be modified slightly to emphasise that the issue is with Miniscript expressions and not only the Taproot descriptor.
πŸ’¬ rkrux commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r2510381532)
Since the issue is not specific to taproot descriptors and instead the ones containing MiniScript expression.

```diff
- self.log.info('Test taproot descriptor do not have mixed hardened derivation marker')
+ self.log.info('Test descriptors do not have mixed hardened derivation marker')
```
πŸ’¬ rkrux commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#discussion_r2510396784)
```diff
wallet = node.get_wallet_rpc('w5')
+ tr_desc = "tr([1dce71b2/48'/1'/0'/2']tpubDEeP3GefjqbaDTTaVAF5JkXWhoFxFDXQ9KuhVrMBViFXXNR2B3Lvme2d2Aoy
iKfzRFZChq2AGMNbU1qTbkBMfNv7WGVXLt2pnYXY87gXqcs/0/*,and_v(v:pk([c658b283/48'/1'/0'/2']tpubDFL5wzgPBYK5pZ2Kh1T8
qrxnp43kjE5CXfguZHHBrZSWpkfASy5rVfj7prh11XdqkC1P3kRwUPBeX7AHN8XBNx8UwiprnFnEm5jyswiRD4p/0/*),older(65535)))"
+ wsh_desc = "wsh(or_d(pk([a233d117/48'/1'/0'/2']tpubDF8d1Q2U8WWfxUHMiqqrYiavBReX2r7hwD7oQsEuq1AiXj5nJc

...
πŸ’¬ stickies-v commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2510459502)
> So the two calls should just be removed imo.

Done.
πŸ’¬ hebasto commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3511454795)
> Any chance you want to turn this into just a subtree pull, and do the switchover in a followup PR?

Sure. The last commit has been dropped.
πŸ’¬ stickies-v commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#issuecomment-3511454747)
Force-pushed to remove the `bitcoinkernel_wrapper.h` `Tip()` and `Genesis()` methods too, as suggested [here](https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2507308573).
πŸ’¬ RandyMcMillan commented on pull request "[wip] wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3511602426)
Concept ACK
πŸ’¬ fanquake commented on pull request "util: Allow `Assert` (et al.) in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2510562617)
Rebased #33775 on this, and dropped the workarounds back out.
πŸ’¬ hodlinator commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2510587958)
Now that #33785 has been merged we hopefully can use `const& ... Assert(` in this PR as long as it's rebased.
πŸ“ waketraindev converted_to_draft a pull request: "Prevent re-execution of sensitive commands from console history"
(https://github.com/bitcoin-core/gui/pull/909)
Sensitive RPC commands such as `walletpassphrase` or `createwallet`
may appear in the console history with their arguments redacted. Previously,
these entries could still be re-executed if recalled, potentially causing
unintended actions.

This change prefixes sensitive history entries with a leading character(`!`),
marking them as non-executable when called. The console blocks their
execution and informs the user that the command was blocked.
The help text in `help-console` has been upd
...
πŸ’¬ hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511766342)
> I guess we are now just blocked on Qt / GUI tooling...

From https://github.com/bitcoin/bitcoin/pull/32648#issuecomment-3027824977:
> this should be fixed in Qts tooling.

Even if it’s been fixed upstream, I don’t expect that change to be backported to all Qt versions down to 6.2. Therefore, we still need to suppress warnings in generated files. For example, as follows:
```diff
--- a/src/qt/CMakeLists.txt
+++ b/src/qt/CMakeLists.txt
@@ -50,11 +50,20 @@ endfunction()

set(CMAKE_AUT
...
πŸ’¬ maflcko commented on pull request "refactor: Add missing include in bitcoinkernel_wrapper.h":
(https://github.com/bitcoin/bitcoin/pull/33825#issuecomment-3511768897)
> [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?
πŸ’¬ fanquake commented on pull request "Update `minisketch` subtree":
(https://github.com/bitcoin/bitcoin/pull/32856#issuecomment-3511834935)
ACK c235aa468b0dcc67b49340dbe9b675c513cec7bf
πŸ€” 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.