π vasild's pull request is ready for review: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326)
(https://github.com/bitcoin/bitcoin/pull/32326)
π¬ hebasto commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844437349)
@laanwj
Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844437349)
@laanwj
Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
π¬ laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844447695)
> Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
Sure. Rebased to master.
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2844447695)
> Before undrafting this PR. could you please rebase it to include the merged https://github.com/bitcoin/bitcoin/pull/32071. Otherwise, NetBSD [tests](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14772257138) fail.
Sure. Rebased to master.
π¬ AndreaLanfranchi commented on pull request "Update .gitignore":
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844448468)
> We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
Thank you for the clarification.
What about instead the default "out" build dir in Windows (VS toolchain) ... that would require changes in presets and **is** IDE and OS dependant.
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844448468)
> We generally do not accept IDE-specific entries in the projectβs `.gitignore`. Please adjust your build environment by other means.
Thank you for the clarification.
What about instead the default "out" build dir in Windows (VS toolchain) ... that would require changes in presets and **is** IDE and OS dependant.
π¬ vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070059886)
Done in latest push.
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2070059886)
Done in latest push.
π fanquake merged a pull request: "doc: Fix test_bitcoin path"
(https://github.com/bitcoin/bitcoin/pull/32389)
(https://github.com/bitcoin/bitcoin/pull/32389)
π hebasto approved a pull request: "doc: Fix test_bitcoin path"
(https://github.com/bitcoin/bitcoin/pull/32389#pullrequestreview-2809473551)
ACK 6cbc28b8dd629062950f195facc009fd8ba86310.
(https://github.com/bitcoin/bitcoin/pull/32389#pullrequestreview-2809473551)
ACK 6cbc28b8dd629062950f195facc009fd8ba86310.
π¬ hebasto commented on pull request "doc: Fix test_bitcoin path":
(https://github.com/bitcoin/bitcoin/pull/32389#issuecomment-2844458990)
Backport to 29.x?
(https://github.com/bitcoin/bitcoin/pull/32389#issuecomment-2844458990)
Backport to 29.x?
π¬ Sjors commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844569676)
@ajtowns yes, that would be even simpler than this PR, but would still run into the above objection:
> Concept NACK. At a minimum, it would need to tally up the total length
(https://github.com/bitcoin/bitcoin/pull/32381#issuecomment-2844569676)
@ajtowns yes, that would be even simpler than this PR, but would still run into the above objection:
> Concept NACK. At a minimum, it would need to tally up the total length
π¬ fanquake commented on issue "test: functional test failures under -DENABLE_WALLET=OFF":
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2844572481)
> could you share the build/test/config.ini file?
```bash
# Copyright (c) 2013-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# These environment variables are set by the build process and read by
# test/*/test_runner.py and test/util/rpcauth-test.py
[environment]
CLIENT_NAME=Bitcoin Core
CLIENT_BUGREPORT=https://github.com/bitcoin/bitcoin/issues
SRCDIR=/root/bitcoin
BUIL
...
(https://github.com/bitcoin/bitcoin/issues/32347#issuecomment-2844572481)
> could you share the build/test/config.ini file?
```bash
# Copyright (c) 2013-2016 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# These environment variables are set by the build process and read by
# test/*/test_runner.py and test/util/rpcauth-test.py
[environment]
CLIENT_NAME=Bitcoin Core
CLIENT_BUGREPORT=https://github.com/bitcoin/bitcoin/issues
SRCDIR=/root/bitcoin
BUIL
...
π¬ Sjors commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2844578785)
@D33r-Gee thanks, I'll take a look!
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2844578785)
@D33r-Gee thanks, I'll take a look!
π¬ Sjors commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2844581124)
Concept ACK
Although I'm not sure how much effort we should put into maintaining the current GUI codebase, this particular feature is high on my wish list. And the code changes here seem very simple. So I'm happy to test and review it.
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2844581124)
Concept ACK
Although I'm not sure how much effort we should put into maintaining the current GUI codebase, this particular feature is high on my wish list. And the code changes here seem very simple. So I'm happy to test and review it.
β οΈ krozk opened an issue: "Hello
My name is krozk. Iβm an independent developer and I've created a highly secured, Bitcoin-integrated codebase designed for exclusive distribution and automation.
This project includes:
- Full source code protection
- One-time buyer licensing (only I can resell)
- Bitcoin payment verification before code access
- Optional proxy, VPN, and Tor integration
I believe this may be of interest to developers and teams focused on security, cryptography, and automation.
Repository: [your GitHub link]
If you're interested in a custom license or demo, feel free to reach out. Iβd be honored to discuss further collaboration or integration.
Thank you for your time.
Best regards,
**krozk**"
(https://github.com/bitcoin/bitcoin/issues/32393)
My name is krozk. Iβm an independent developer and I've created a highly secured, Bitcoin-integrated codebase designed for exclusive distribution and automation.
This project includes:
- Full source code protection
- One-time buyer licensing (only I can resell)
- Bitcoin payment verification before code access
- Optional proxy, VPN, and Tor integration
I believe this may be of interest to developers and teams focused on security, cryptography, and automation.
Repository: [your GitHub link]
If you're interested in a custom license or demo, feel free to reach out. Iβd be honored to discuss further collaboration or integration.
Thank you for your time.
Best regards,
**krozk**"
(https://github.com/bitcoin/bitcoin/issues/32393)
β
willcl-ark closed an issue: "Hello
My name is krozk. Iβm an independent developer and I've created a highly secured, Bitcoin-integrated codebase designed for exclusive distribution and automation.
This project includes:
- Full source code protection
- One-time buyer licensing (only I can resell)
- Bitcoin payment verification before code access
- Optional proxy, VPN, and Tor integration
I believe this may be of interest to developers and teams focused on security, cryptography, and automation.
Repository: [your GitHub link]
If you're interested in a custom license or demo, feel free to reach out. Iβd be honored to discuss further collaboration or integration.
Thank you for your time.
Best regards,
**krozk**"
(https://github.com/bitcoin/bitcoin/issues/32393)
My name is krozk. Iβm an independent developer and I've created a highly secured, Bitcoin-integrated codebase designed for exclusive distribution and automation.
This project includes:
- Full source code protection
- One-time buyer licensing (only I can resell)
- Bitcoin payment verification before code access
- Optional proxy, VPN, and Tor integration
I believe this may be of interest to developers and teams focused on security, cryptography, and automation.
Repository: [your GitHub link]
If you're interested in a custom license or demo, feel free to reach out. Iβd be honored to discuss further collaboration or integration.
Thank you for your time.
Best regards,
**krozk**"
(https://github.com/bitcoin/bitcoin/issues/32393)
π¬ vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844588105)
`75acdcbaa9...15cc989538`: incorporate feedback from above
I took your branch https://github.com/theuni/bitcoin/commits/finalize_threadheandler2/ and made some changes:
**No need to ping-pong the tear down of a `CNode` between the `net` and `msghand` threads so much**
In [finalize_threadheandler2](https://github.com/theuni/bitcoin/commits/finalize_threadheandler2/):
1. The `net` thread would disconnect a node and move it from `m_nodes` to `m_nodes_disconnected`.
2. Then the `msghand
...
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844588105)
`75acdcbaa9...15cc989538`: incorporate feedback from above
I took your branch https://github.com/theuni/bitcoin/commits/finalize_threadheandler2/ and made some changes:
**No need to ping-pong the tear down of a `CNode` between the `net` and `msghand` threads so much**
In [finalize_threadheandler2](https://github.com/theuni/bitcoin/commits/finalize_threadheandler2/):
1. The `net` thread would disconnect a node and move it from `m_nodes` to `m_nodes_disconnected`.
2. Then the `msghand
...
π€ janb84 reviewed a pull request: "doc: Add missing top-level description to pruneblockchain RPC"
(https://github.com/bitcoin/bitcoin/pull/32333#pullrequestreview-2809563698)
ACK [fa5ed18](https://github.com/bitcoin/bitcoin/pull/32333/commits/fa5ed18946858c837c65b4fa730fbeda780c68e2)
This PR adds some additional explanatory text (top level) to the `pruneblockchain` RPC help output. Provides more clarity what the command does and makes it more consistent with other help outputs.
New output :
```shell
% ./bitcoin-cli --regtest pruneblockchain
error code: -1
error message:
pruneblockchain height
Deletes block and undo data up to a specified height or ti
...
(https://github.com/bitcoin/bitcoin/pull/32333#pullrequestreview-2809563698)
ACK [fa5ed18](https://github.com/bitcoin/bitcoin/pull/32333/commits/fa5ed18946858c837c65b4fa730fbeda780c68e2)
This PR adds some additional explanatory text (top level) to the `pruneblockchain` RPC help output. Provides more clarity what the command does and makes it more consistent with other help outputs.
New output :
```shell
% ./bitcoin-cli --regtest pruneblockchain
error code: -1
error message:
pruneblockchain height
Deletes block and undo data up to a specified height or ti
...
π¬ laanwj commented on pull request "Update .gitignore":
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844593526)
See https://docs.github.com/en/get-started/git-basics/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer
(https://github.com/bitcoin/bitcoin/pull/32392#issuecomment-2844593526)
See https://docs.github.com/en/get-started/git-basics/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer
π¬ vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844593964)
> Can we maybe just hold off on this for now? The investigation above prompted me to start working on a much larger refactor...
Not sure about that because I have not seen that other refactor. In general I prefer do stuff in smaller, more manageable steps. I guess a further refactor should be possible on top of this PR. I would be happy to review. This PR should make a more solid foundation for further changes in the code.
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2844593964)
> Can we maybe just hold off on this for now? The investigation above prompted me to start working on a much larger refactor...
Not sure about that because I have not seen that other refactor. In general I prefer do stuff in smaller, more manageable steps. I guess a further refactor should be possible on top of this PR. I would be happy to review. This PR should make a more solid foundation for further changes in the code.
π l0rinc approved a pull request: "validation: write chainstate to disk every hour"
(https://github.com/bitcoin/bitcoin/pull/30611#pullrequestreview-2809508852)
ACK e976bd3045010ee217aa0f2dca4c962aabb789d5
I'm glad to see this moving along, this will help other optimizations as well by eliminating the worst-case scenario of crashing at the end of IBD with an OOM.
I'm fine with merging as it is, I only have nits and minor suggestions - but will glady reack if they're applied.
I've retested it locally by running all tests and doing a few thousand blocks of IBD in debug mode (without any changes and with modified `1min`..`2min` interval to trigger
...
(https://github.com/bitcoin/bitcoin/pull/30611#pullrequestreview-2809508852)
ACK e976bd3045010ee217aa0f2dca4c962aabb789d5
I'm glad to see this moving along, this will help other optimizations as well by eliminating the worst-case scenario of crashing at the end of IBD with an OOM.
I'm fine with merging as it is, I only have nits and minor suggestions - but will glady reack if they're applied.
I've retested it locally by running all tests and doing a few thousand blocks of IBD in debug mode (without any changes and with modified `1min`..`2min` interval to trigger
...
π¬ l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2070087705)
we can inline `mNow`:
```suggestion
const bool fPeriodicWrite{mode == FlushStateMode::PERIODIC && NodeClock::now() >= m_next_write};
```
nit: maybe we could add some cleanup commit here which uses brace inits (to avoid all the `=` vs `==` confusion here) and to unify the names of these flag variables
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r2070087705)
we can inline `mNow`:
```suggestion
const bool fPeriodicWrite{mode == FlushStateMode::PERIODIC && NodeClock::now() >= m_next_write};
```
nit: maybe we could add some cleanup commit here which uses brace inits (to avoid all the `=` vs `==` confusion here) and to unify the names of these flag variables