π¬ hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512348200)
> > Even if itβs been fixed upstream,
>
> It's not clear to me if it has been fixed or not, can you link to the relevant change / issue?
See https://bugreports.qt.io/browse/QTBUG-141858.
> I can cherry-pick the change above in, if you create a commit, and add a comment inline, for when the change can be dropped.
Feel free to pick top two commits from https://github.com/hebasto/bitcoin/commits/pr32482/1110/.
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512348200)
> > Even if itβs been fixed upstream,
>
> It's not clear to me if it has been fixed or not, can you link to the relevant change / issue?
See https://bugreports.qt.io/browse/QTBUG-141858.
> I can cherry-pick the change above in, if you create a commit, and add a comment inline, for when the change can be dropped.
Feel free to pick top two commits from https://github.com/hebasto/bitcoin/commits/pr32482/1110/.
π¬ fanquake commented on pull request "test: add tests for private broadcast":
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512354999)
Concept NACK to deferring adding tests until after adding a new feature.
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512354999)
Concept NACK to deferring adding tests until after adding a new feature.
π€ hebasto reviewed a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3443882743)
Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
```
sudo apt-get install build-essential ...
cmake --build build
```
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3443882743)
Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
```
sudo apt-get install build-essential ...
cmake --build build
```
β
vasild closed a pull request: "test: add tests for private broadcast"
(https://github.com/bitcoin/bitcoin/pull/33843)
(https://github.com/bitcoin/bitcoin/pull/33843)
π¬ vasild commented on pull request "test: add tests for private broadcast":
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512512550)
Closing after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236
(https://github.com/bitcoin/bitcoin/pull/33843#issuecomment-3512512550)
Closing after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512531635)
`d419fbc42b..29ef3c62de`: restore back the test here and close https://github.com/bitcoin/bitcoin/pull/33843 after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236
If people review up to but not including the test, they can ACK the last but one commit.
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3512531635)
`d419fbc42b..29ef3c62de`: restore back the test here and close https://github.com/bitcoin/bitcoin/pull/33843 after discussion at https://www.erisian.com.au/bitcoin-core-dev/log-2025-11-10.html#l-236
If people review up to but not including the test, they can ACK the last but one commit.
π¬ fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512546980)
> See https://bugreports.qt.io/browse/QTBUG-141858.
> Feel free to pick
Thanks. I've pulled those two in here.
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512546980)
> See https://bugreports.qt.io/browse/QTBUG-141858.
> Feel free to pick
Thanks. I've pulled those two in here.
π¬ achow101 commented on pull request "wallet: Always rewrite tx records during migration":
(https://github.com/bitcoin/bitcoin/pull/32985#issuecomment-3512556430)
> these changes are persisted during `ReorderTransactions()`
Not for migration since legacy wallets are only opened with a `BerkeleyRODatabase` which does not write anything.
(https://github.com/bitcoin/bitcoin/pull/32985#issuecomment-3512556430)
> these changes are persisted during `ReorderTransactions()`
Not for migration since legacy wallets are only opened with a `BerkeleyRODatabase` which does not write anything.
π¬ maflcko commented on pull request "build: Bump g++ minimum supported version to 12":
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3512575639)
> Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
I am happy to add a general note that the latest stable or LTS is recommended and that earlier releases may need the user to install the required minimum dependencies.
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3512575639)
> Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
I am happy to add a general note that the latest stable or LTS is recommended and that earlier releases may need the user to install the required minimum dependencies.
π nichechristie opened a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
β
hebasto closed a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
(https://github.com/bitcoin/bitcoin/pull/33844)
π€ janb84 reviewed a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9
Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.
steps taken:
code review,
build,
ran all tests,
(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9
Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.
steps taken:
code review,
build,
ran all tests,
(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...
π¬ janb84 commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```
Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```
Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...
π¬ achow101 commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
π maflcko opened a pull request: "ci: Enable experimental kernel stuff in ASan task"
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:
```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:
```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065
π achow101 merged a pull request: "log: reduce excessive "rolling back/forward" messages during block replay"
(https://github.com/bitcoin/bitcoin/pull/33443)
(https://github.com/bitcoin/bitcoin/pull/33443)
π¬ maflcko commented on pull request "ci: Enable experimental kernel stuff in ASan task":
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from 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?
Yes, either should be fine. To clarif
...
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from 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?
Yes, either should be fine. To clarif
...
π¬ maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
π¬ achow101 commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
π¬ kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)