💬 yancyribbens commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2496069524)
Ey thanks for catching that! I updated the test so that it re-populates the `doppelgangers` array. It's tricky that the api doesn't return a different result when the TOTAL_TRIES is exceeded.
I maintain a rust version of coin-grinder, and I added a test that is the same as here: https://github.com/p2pderivatives/rust-bitcoin-coin-selection/commit/a0e033b67ba20139472075a9fc428a1a81c357e7 . You'll notice in the rust version, one can actually assert `expected_error: Some(IterationLimitReached
...
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2496069524)
Ey thanks for catching that! I updated the test so that it re-populates the `doppelgangers` array. It's tricky that the api doesn't return a different result when the TOTAL_TRIES is exceeded.
I maintain a rust version of coin-grinder, and I added a test that is the same as here: https://github.com/p2pderivatives/rust-bitcoin-coin-selection/commit/a0e033b67ba20139472075a9fc428a1a81c357e7 . You'll notice in the rust version, one can actually assert `expected_error: Some(IterationLimitReached
...
💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2496260062)
Thanks! The comment has been restored and adjusted.
(https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2496260062)
Thanks! The comment has been restored and adjusted.
💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3493631585)
The [feedback](https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2494445665) from @ryanofsky has been addressed.
Additionally, `PROJECT_SOURCE_DIR` were replaced with `CMAKE_CURRENT_SOURCE_DIR`.
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3493631585)
The [feedback](https://github.com/bitcoin/bitcoin/pull/33774#discussion_r2494445665) from @ryanofsky has been addressed.
Additionally, `PROJECT_SOURCE_DIR` were replaced with `CMAKE_CURRENT_SOURCE_DIR`.
📝 kevkevinpal opened a pull request: "test: skip interface_ipc if python version is freethreaded and PYTHON_GIL=0 is not set"
(https://github.com/bitcoin/bitcoin/pull/33795)
### Description
I was able to reproduce this issue https://github.com/bitcoin/bitcoin/issues/33582
We should skip the test if `PYTHON_GIL` is not set, instead of the test being abruptly stopped
### When `PYTHON_GIL` not set or set to `1`
```
PYTHON_GIL=1 ./build_dev_mode/test/functional/test_runner.py interface_ipc.py
or
./build_dev_mode/test/functional/test_runner.py interface_ipc.py
Temporary test directory at /tmp/test_runner_₿_🏃_20251105_180439
WARNING! There is already a bitc
...
(https://github.com/bitcoin/bitcoin/pull/33795)
### Description
I was able to reproduce this issue https://github.com/bitcoin/bitcoin/issues/33582
We should skip the test if `PYTHON_GIL` is not set, instead of the test being abruptly stopped
### When `PYTHON_GIL` not set or set to `1`
```
PYTHON_GIL=1 ./build_dev_mode/test/functional/test_runner.py interface_ipc.py
or
./build_dev_mode/test/functional/test_runner.py interface_ipc.py
Temporary test directory at /tmp/test_runner_₿_🏃_20251105_180439
WARNING! There is already a bitc
...
📝 w0xlt opened a pull request: "[kernel] Expose `CheckTransaction` consensus validation function"
(https://github.com/bitcoin/bitcoin/pull/33796)
This PR exposes the consensus-level `CheckTransaction` function through the libbitcoinkernel C API and adds a corresponding C++ wrapper.
Currently, libkernel only provided script-level validation via `btck_script_pubkey_verify` and `ScriptPubkeyApi<>::Verify`.
AFAIK there was no way to perform context-free consensus checks on a transaction’s structure (e.g., coinbase rules, money-range, duplicate inputs).
This change introduces a new API:
```c
int btck_check_transaction(const btck_
...
(https://github.com/bitcoin/bitcoin/pull/33796)
This PR exposes the consensus-level `CheckTransaction` function through the libbitcoinkernel C API and adds a corresponding C++ wrapper.
Currently, libkernel only provided script-level validation via `btck_script_pubkey_verify` and `ScriptPubkeyApi<>::Verify`.
AFAIK there was no way to perform context-free consensus checks on a transaction’s structure (e.g., coinbase rules, money-range, duplicate inputs).
This change introduces a new API:
```c
int btck_check_transaction(const btck_
...
✅ w0xlt closed a pull request: "Add libbitcoinkernel example files"
(https://github.com/bitcoin/bitcoin/pull/33669)
(https://github.com/bitcoin/bitcoin/pull/33669)
💬 w0xlt commented on pull request "Add libbitcoinkernel example files":
(https://github.com/bitcoin/bitcoin/pull/33669#issuecomment-3494136470)
Closing this PR for now.
If there’s further interest in adding these examples to this project, we can reopen it.
(https://github.com/bitcoin/bitcoin/pull/33669#issuecomment-3494136470)
Closing this PR for now.
If there’s further interest in adding these examples to this project, we can reopen it.
💬 pablomartin4btc commented on pull request "doc: add cmake help option in Windows build docs":
(https://github.com/bitcoin/bitcoin/pull/33789#discussion_r2496576852)
Not necessarily, in fact, most build instructions have different structure (unify it, if possible, could be the goal of a different PR, as you can see here we need to copy stuff for every platform). My suggestion is not a blocker just that you are adding this line in the middle of the building instructions, and that's not the case for the other file you are updating. It's an observation, up to you if you want to leave it that way or not.
(https://github.com/bitcoin/bitcoin/pull/33789#discussion_r2496576852)
Not necessarily, in fact, most build instructions have different structure (unify it, if possible, could be the goal of a different PR, as you can see here we need to copy stuff for every platform). My suggestion is not a blocker just that you are adding this line in the middle of the building instructions, and that's not the case for the other file you are updating. It's an observation, up to you if you want to leave it that way or not.
🤔 w0xlt reviewed a pull request: "test: move create_malleated_version() to messages.py for reuse"
(https://github.com/bitcoin/bitcoin/pull/33793#pullrequestreview-3425046399)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/33793/commits/2bd155e6ee7e3cabd76083ac921b34bb45d98769
(https://github.com/bitcoin/bitcoin/pull/33793#pullrequestreview-3425046399)
Code Review ACK https://github.com/bitcoin/bitcoin/pull/33793/commits/2bd155e6ee7e3cabd76083ac921b34bb45d98769
📝 mccoystevens78-web opened a pull request: "Create devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/33797)
<!--
*** 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/33797)
<!--
*** 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
...
📝 mccoystevens78-web opened a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/33798)
<!--
*** 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/33798)
<!--
*** 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
...
📝 mccoystevens78-web opened a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/33799)
<!--
*** 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/33799)
<!--
*** 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
...
📝 amitsingh881024-ctrl opened a pull request: "Create SECURITY.md — Strong Security Policy"
(https://github.com/bitcoin/bitcoin/pull/33800)
# Security Policy
## 🔒 Supported Versions
Please refer to the official lifecycle page to identify supported and actively maintained releases:
- [https://bitcoincore.org/en/lifecycle/#schedule](https://bitcoincore.org/en/lifecycle/#schedule)
Security updates are provided only for the listed maintained versions. Users are **strongly advised to upgrade** to supported versions to receive patches.
---
## 🚨 Reporting a Vulnerability
If you discover a security vulnerability, please
...
(https://github.com/bitcoin/bitcoin/pull/33800)
# Security Policy
## 🔒 Supported Versions
Please refer to the official lifecycle page to identify supported and actively maintained releases:
- [https://bitcoincore.org/en/lifecycle/#schedule](https://bitcoincore.org/en/lifecycle/#schedule)
Security updates are provided only for the listed maintained versions. Users are **strongly advised to upgrade** to supported versions to receive patches.
---
## 🚨 Reporting a Vulnerability
If you discover a security vulnerability, please
...
💬 amitsingh881024-ctrl commented on pull request "Create SECURITY.md — Strong Security Policy":
(https://github.com/bitcoin/bitcoin/pull/33800#issuecomment-3494370269)
curl -X POST \
-H "Authorization: token YOUR_GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
https://api.github.com/repos/OWNER/REPO/issues/PR_NUMBER/comments \
-d '{"body":"Please update the README before merging."}'
(https://github.com/bitcoin/bitcoin/pull/33800#issuecomment-3494370269)
curl -X POST \
-H "Authorization: token YOUR_GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
https://api.github.com/repos/OWNER/REPO/issues/PR_NUMBER/comments \
-d '{"body":"Please update the README before merging."}'
💬 amitsingh881024-ctrl commented on pull request "Create SECURITY.md — Strong Security Policy":
(https://github.com/bitcoin/bitcoin/pull/33800#issuecomment-3494371111)
curl -X POST \
-H "Authorization: token YOUR_GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
https://api.github.com/repos/OWNER/REPO/issues/PR_NUMBER/comments \
-d '{"body":"Please update the README before merging."}'
(https://github.com/bitcoin/bitcoin/pull/33800#issuecomment-3494371111)
curl -X POST \
-H "Authorization: token YOUR_GITHUB_TOKEN" \
-H "Accept: application/vnd.github+json" \
https://api.github.com/repos/OWNER/REPO/issues/PR_NUMBER/comments \
-d '{"body":"Please update the README before merging."}'
💬 w0xlt commented on pull request "rpc: Optionally print feerates in sat/vb":
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496724101)
This drags an RPC‑only dependency (UniValue) into a widely included policy header.
Instead, a helper in `rpc/util.{h,cpp}` (or `rpc/fees.h`) could be added such as:
```cpp
enum class FeeRateUnit { BTC_KVB, SAT_VB };
UniValue ValueFromFeeRate(const CFeeRate& fr, FeeRateUnit unit);
```
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496724101)
This drags an RPC‑only dependency (UniValue) into a widely included policy header.
Instead, a helper in `rpc/util.{h,cpp}` (or `rpc/fees.h`) could be added such as:
```cpp
enum class FeeRateUnit { BTC_KVB, SAT_VB };
UniValue ValueFromFeeRate(const CFeeRate& fr, FeeRateUnit unit);
```
💬 w0xlt commented on pull request "rpc: Optionally print feerates in sat/vb":
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496731465)
Do the last six lines duplicate three assertions ?
You probably intended to add negative sat/vB cases.
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496731465)
Do the last six lines duplicate three assertions ?
You probably intended to add negative sat/vB cases.
💬 murchandamus commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2496738958)
> Removing UTXO's with the lowest effective_value minimizes the number of UTXOS included in the result when the maximum weight is exceeded.
Maybe we have diverging understandings of what the verb “minimize” means. I understand it to mean “reduce to a minimum”. Are you using it as a synonym for “reduce”? Or do you think that removing a single OutputGroup always reduces the selection to the minimum necessary selection?
Because, per my understanding of the verb, this algorithm does not _minim
...
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2496738958)
> Removing UTXO's with the lowest effective_value minimizes the number of UTXOS included in the result when the maximum weight is exceeded.
Maybe we have diverging understandings of what the verb “minimize” means. I understand it to mean “reduce to a minimum”. Are you using it as a synonym for “reduce”? Or do you think that removing a single OutputGroup always reduces the selection to the minimum necessary selection?
Because, per my understanding of the verb, this algorithm does not _minim
...
💬 w0xlt commented on pull request "rpc: Optionally print feerates in sat/vb":
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496744390)
This line constructs a full object (with locking) only to reuse two values.
It also adds a new include `<rpc/mempool.h>` into `rpc/net.cpp`, creating an RPC‑RPC dependency.
It may be better to compute these two fields directly.
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2496744390)
This line constructs a full object (with locking) only to reuse two values.
It also adds a new include `<rpc/mempool.h>` into `rpc/net.cpp`, creating an RPC‑RPC dependency.
It may be better to compute these two fields directly.
⚠️ Vashan69 opened an issue: "VLZO COIN"
(https://github.com/bitcoin/bitcoin/issues/33801)
### Please describe the feature you'd like to see added.
<img width="500" height="500" alt="Image" src="https://github.com/user-attachments/assets/e229fa17-1a3b-4fdc-ba8f-c3f0068840ac" />
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
<img width="4269" height="2400" alt="Image" src="https://github.com/user-attachments/assets/234cadb2-fef4-4d73-aa58-e2f392428dd9" />
### Describe any alternatives you've considered
_No r
...
(https://github.com/bitcoin/bitcoin/issues/33801)
### Please describe the feature you'd like to see added.
<img width="500" height="500" alt="Image" src="https://github.com/user-attachments/assets/e229fa17-1a3b-4fdc-ba8f-c3f0068840ac" />
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
<img width="4269" height="2400" alt="Image" src="https://github.com/user-attachments/assets/234cadb2-fef4-4d73-aa58-e2f392428dd9" />
### Describe any alternatives you've considered
_No r
...