💬 fanquake commented on pull request "scripted-diff: Remove obsolete comment":
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3510884754)
Backported to `30.x` in #33609.
(https://github.com/bitcoin/bitcoin/pull/33826#issuecomment-3510884754)
Backported to `30.x` in #33609.
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510056357)
From what I can infer, we use `MinimumChainWork` to set an "expected chain length". This protects the node from any chain that is not at least as good as the set expected chain length, especially when it is eclipsed. Since the assumevalid block is typically set at the `MinimumChainWork` height, it would seem that we can remove the `MinimumChainWork` check from here by checking that the assumevalid block is an ancestor of (or is) the best_header, but in the case we are using a malicious assumeva
...
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510056357)
From what I can infer, we use `MinimumChainWork` to set an "expected chain length". This protects the node from any chain that is not at least as good as the set expected chain length, especially when it is eclipsed. Since the assumevalid block is typically set at the `MinimumChainWork` height, it would seem that we can remove the `MinimumChainWork` check from here by checking that the assumevalid block is an ancestor of (or is) the best_header, but in the case we are using a malicious assumeva
...
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510065160)
I wouldn't mind adding a comment stating this. Reviewers can confirm if it correctly reflects the intent of the code.
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2510065160)
I wouldn't mind adding a comment stating this. Reviewers can confirm if it correctly reflects the intent of the code.
💬 willcl-ark commented on issue "Inconsistent CJDNS address handling in Local addresses and AddLocal logs":
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3511034888)
@big14way you don't need to be assigned an issue to work on it in this project. Opening a pull request (please read CONTRIBUTING.md frist) and linking it to this issue in the PR description is all that's needed.
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3511034888)
@big14way you don't need to be assigned an issue to work on it in this project. Opening a pull request (please read CONTRIBUTING.md frist) and linking it to this issue in the PR description is all that's needed.
💬 TheCharlatan commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065)
> I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065)
> I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
💬 l0rinc commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511133487)
reACK 2e713c22c67b3551f04273428db266503178e7fa
<details>
<summary>change since last ack</summary>
```bash
git fetch origin 2e713c22c67b3551f04273428db266503178e7fa && git range-diff 854269a...2e713c2
```
commit messages include scripted diffs and the suggested change by AJ:
```
+%define TBL rbp
```
</details>
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511133487)
reACK 2e713c22c67b3551f04273428db266503178e7fa
<details>
<summary>change since last ack</summary>
```bash
git fetch origin 2e713c22c67b3551f04273428db266503178e7fa && git range-diff 854269a...2e713c2
```
commit messages include scripted diffs and the suggested change by AJ:
```
+%define TBL rbp
```
</details>
💬 hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2510211223)
This change has just been merged [upstream](https://github.com/arun11299/cpp-subprocess/pull/121).
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2510211223)
This change has just been merged [upstream](https://github.com/arun11299/cpp-subprocess/pull/121).
💬 fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511165591)
> if expand/sponge are available:
We don't have sponge, but added a commit to add `moreutils`, and pushed up the scripted diffs.
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3511165591)
> if expand/sponge are available:
We don't have sponge, but added a commit to add `moreutils`, and pushed up the scripted diffs.
💬 fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2510228737)
Thanks.
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2510228737)
Thanks.
🚀 fanquake merged a pull request: "util: Allow `Assert` (et al.) in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785)
(https://github.com/bitcoin/bitcoin/pull/33785)
💬 yuvicc commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2510258822)
This sounds good to me. Maybe can be done on a separate PR?
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2510258822)
This sounds good to me. Maybe can be done on a separate PR?
🤔 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
...
(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...
(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?
(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.
(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')
```
(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
...
(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.
(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.
(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).
(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).