π¬ caesrcd commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092509339)
The mempool is already constrained by `DEFAULT_MAX_MEMPOOL_SIZE_MB`, which prevents unbounded transaction accumulation and significantly reduces the feasibility of relay-based DoS attacks. Historically, the network has operated reliably even during extended periods of mempool congestion, where dynamic fee pressure naturally evicts lower-fee transactions.
This proposed configuration provides a practical balance between usability and relay robustness:
```cpp
static constexpr unsigned int DEFA
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092509339)
The mempool is already constrained by `DEFAULT_MAX_MEMPOOL_SIZE_MB`, which prevents unbounded transaction accumulation and significantly reduces the feasibility of relay-based DoS attacks. Historically, the network has operated reliably even during extended periods of mempool congestion, where dynamic fee pressure naturally evicts lower-fee transactions.
This proposed configuration provides a practical balance between usability and relay robustness:
```cpp
static constexpr unsigned int DEFA
...
π¬ Aditya-PS-05 commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092519691)
> > here, the max_tip_age defaults to 24 hours. When a node is offline for "a few days" and restarts ~600 blocks behind, the tip is older than 24 hours, incorrectly triggering IBD mode.
>
> It _should_ be in IBD mode in this case.
You're absolutely right that IBD mode should sync.
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092519691)
> > here, the max_tip_age defaults to 24 hours. When a node is offline for "a few days" and restarts ~600 blocks behind, the tip is older than 24 hours, incorrectly triggering IBD mode.
>
> It _should_ be in IBD mode in this case.
You're absolutely right that IBD mode should sync.
π hodlinator approved a pull request: "refactor: GenTxid type safety followups"
(https://github.com/bitcoin/bitcoin/pull/33005#pullrequestreview-3035484779)
crACK e991cac062ca644f3c4ac5c5144eb0daf445fd09
Thanks for doing the follow-up!
The major change is that `Peer::TxRelay::m_tx_inventory_to_send` now uses `Wtxid` instead of `GenTxid` while the `Peer::TxRelay::m_tx_inventory_known_filter` bloom filter still uses either `Wtxid` or `Txid` consistently depending on the type of peer.
Only one style-nit in case you re-touch.
(https://github.com/bitcoin/bitcoin/pull/33005#pullrequestreview-3035484779)
crACK e991cac062ca644f3c4ac5c5144eb0daf445fd09
Thanks for doing the follow-up!
The major change is that `Peer::TxRelay::m_tx_inventory_to_send` now uses `Wtxid` instead of `GenTxid` while the `Peer::TxRelay::m_tx_inventory_known_filter` bloom filter still uses either `Wtxid` or `Txid` consistently depending on the type of peer.
Only one style-nit in case you re-touch.
π¬ hodlinator commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2217409016)
nit: Could be made more consistent with:
https://github.com/bitcoin/bitcoin/blob/e991cac062ca644f3c4ac5c5144eb0daf445fd09/src/net_processing.cpp#L5815-L5817
<details><summary>(Personally I prefer 1 branch and curly-braces).</summary>
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index ebb3db39a5..f38dc4b90d 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -5758,10 +5758,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
...
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2217409016)
nit: Could be made more consistent with:
https://github.com/bitcoin/bitcoin/blob/e991cac062ca644f3c4ac5c5144eb0daf445fd09/src/net_processing.cpp#L5815-L5817
<details><summary>(Personally I prefer 1 branch and curly-braces).</summary>
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index ebb3db39a5..f38dc4b90d 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -5758,10 +5758,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
...
π¬ Aditya-PS-05 commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092524816)
It should be in sync but I guess it can't due to highly restrictive block download logic. IBD mode's overly restrictive block download logic https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L5915
prevents syncing when no preferred download peers are available, causing complete sync failure despite having connected peers.
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092524816)
It should be in sync but I guess it can't due to highly restrictive block download logic. IBD mode's overly restrictive block download logic https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L5915
prevents syncing when no preferred download peers are available, causing complete sync failure despite having connected peers.
π andrewtoth opened a pull request: "coins: remove SetFresh method from CCoinsCacheEntry"
(https://github.com/bitcoin/bitcoin/pull/33018)
There is plenty of test code that exercises the code path for FRESH-but-not-DIRTY coins in CCoinsViewCache. However, we can see there is only one place in production code where we call `SetFresh` that is not preceded by `SetDirty`. This is in `CCoinsViewCache::FetchCoin` and we can see that this is called if the coin retrieved from `base->GetCoin` is spent. The `base` in this case can be another `CCoinsViewCache` or a `CCoinsViewDB`. In `CCoinsViewCache::GetCoin` we can see that we do not return
...
(https://github.com/bitcoin/bitcoin/pull/33018)
There is plenty of test code that exercises the code path for FRESH-but-not-DIRTY coins in CCoinsViewCache. However, we can see there is only one place in production code where we call `SetFresh` that is not preceded by `SetDirty`. This is in `CCoinsViewCache::FetchCoin` and we can see that this is called if the coin retrieved from `base->GetCoin` is spent. The `base` in this case can be another `CCoinsViewCache` or a `CCoinsViewDB`. In `CCoinsViewCache::GetCoin` we can see that we do not return
...
β
andrewtoth closed a pull request: "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries"
(https://github.com/bitcoin/bitcoin/pull/30673)
(https://github.com/bitcoin/bitcoin/pull/30673)
π¬ andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#issuecomment-3092527152)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/33018.
(https://github.com/bitcoin/bitcoin/pull/30673#issuecomment-3092527152)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/33018.
π€ naiyoma reviewed a pull request: "test: add logging to mock external signers"
(https://github.com/bitcoin/bitcoin/pull/32928#pullrequestreview-3035531241)
Concept ACK ->
I attempted to debug -> https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325 after pulling this branch, and so far I can see some useful logs in `combined.log`
<details>
<summary>mock_external_signerβ logs from this <a href="https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325">diff</a></summary>
grep "mock_external_signer" combined.log
node1 2025-07-19T19:35:11.455260Z [init] [common/args.cpp:850] [logArgsPrefix] Command-line arg:
...
(https://github.com/bitcoin/bitcoin/pull/32928#pullrequestreview-3035531241)
Concept ACK ->
I attempted to debug -> https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325 after pulling this branch, and so far I can see some useful logs in `combined.log`
<details>
<summary>mock_external_signerβ logs from this <a href="https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325">diff</a></summary>
grep "mock_external_signer" combined.log
node1 2025-07-19T19:35:11.455260Z [init] [common/args.cpp:850] [logArgsPrefix] Command-line arg:
...
π¬ naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092562076)
>As stated in that comment, one possible reason is that the close of `err_wr_pipe`, which should send the empty string to stdin for the python process (signer.py) to confirm, could return `-1` but bitcoind didn't check the return result. `subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`
>
I am not sure about this because on my end this subprocess works okay
```
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
fprint
...
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092562076)
>As stated in that comment, one possible reason is that the close of `err_wr_pipe`, which should send the empty string to stdin for the python process (signer.py) to confirm, could return `-1` but bitcoind didn't check the return result. `subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`
>
I am not sure about this because on my end this subprocess works okay
```
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
fprint
...
β
bigshiny90 closed a pull request: "test: Add functional tests for blockreconstructionextratxn"
(https://github.com/bitcoin/bitcoin/pull/33016)
(https://github.com/bitcoin/bitcoin/pull/33016)
β οΈ totdking opened an issue: "A missing import to the src/chainparamsbase.h"
(https://github.com/bitcoin/bitcoin/issues/33019)
Tried running the make -j $CORES as the bitcoin repo is important to what I'm working with.
The import that is missing is the ```#include <cstdint>```, it gave a plethora of errors.
This is not a bug but i just wanted the community to have a peaceful and seamless integration while building the repo
(https://github.com/bitcoin/bitcoin/issues/33019)
Tried running the make -j $CORES as the bitcoin repo is important to what I'm working with.
The import that is missing is the ```#include <cstdint>```, it gave a plethora of errors.
This is not a bug but i just wanted the community to have a peaceful and seamless integration while building the repo
π¬ naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092571841)
> Likely the stdin is missing in this particular run and caused `test/functional/mocks/signer.py` to hang.
I agree with this
in `RunCommandParseJSON` ` str_std_in is empty `, I logged this
```
if(str_std_in.empty()) {
LogPrintf("RunCommandParseJSON: No stdin data to send\n");
}
```
logs
```
grep "RunCommandParseJSON" /tmp/bitcoin_func_test_09pw1wgg/node1/regtest/debug.log
2025-07-19T20:52:09.711192Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommand
...
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092571841)
> Likely the stdin is missing in this particular run and caused `test/functional/mocks/signer.py` to hang.
I agree with this
in `RunCommandParseJSON` ` str_std_in is empty `, I logged this
```
if(str_std_in.empty()) {
LogPrintf("RunCommandParseJSON: No stdin data to send\n");
}
```
logs
```
grep "RunCommandParseJSON" /tmp/bitcoin_func_test_09pw1wgg/node1/regtest/debug.log
2025-07-19T20:52:09.711192Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommand
...
π naiyoma opened a pull request: "2025 2/test wallet signer"
(https://github.com/bitcoin/bitcoin/pull/33020)
I noticed that some test cases had been commented out in wallet_signer.py.
This PR uncomments and modifies some of those test cases so that they can pass.
https://github.com/bitcoin/bitcoin/commit/dac74c69491c3787c63b7bf09a3818ea79f9b8a8 -> deleting this assertion since multiple signers is being tested here https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_signer.py#L273
https://github.com/bitcoin/bitcoin/commit/1707c905661ef43935eaaeff06e8141f1be5ea07 test when an
...
(https://github.com/bitcoin/bitcoin/pull/33020)
I noticed that some test cases had been commented out in wallet_signer.py.
This PR uncomments and modifies some of those test cases so that they can pass.
https://github.com/bitcoin/bitcoin/commit/dac74c69491c3787c63b7bf09a3818ea79f9b8a8 -> deleting this assertion since multiple signers is being tested here https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_signer.py#L273
https://github.com/bitcoin/bitcoin/commit/1707c905661ef43935eaaeff06e8141f1be5ea07 test when an
...
π€ djs19901 reviewed a pull request: "depends: add missing Darwin objcopy"
(https://github.com/bitcoin/bitcoin/pull/31840#pullrequestreview-3035634094)
bc1q5vef369tqlwztc08rwjjn3y6986ds56z3hfgg7
(https://github.com/bitcoin/bitcoin/pull/31840#pullrequestreview-3035634094)
bc1q5vef369tqlwztc08rwjjn3y6986ds56z3hfgg7
π l0rinc opened a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OKβLARGEβCRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021)
After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` always end the test early, see:
https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html
The test revival was extracted from a related PR where it was discovered, see: https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797
(https://github.com/bitcoin/bitcoin/pull/33021)
After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` always end the test early, see:
https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html
The test revival was extracted from a related PR where it was discovered, see: https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797
π¬ l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2217521200)
I have extracted the suggested test to a separate PR: https://github.com/bitcoin/bitcoin/pull/33021
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2217521200)
I have extracted the suggested test to a separate PR: https://github.com/bitcoin/bitcoin/pull/33021
π l0rinc approved a pull request: "log: [refactor] Use info level for init logs"
(https://github.com/bitcoin/bitcoin/pull/32967#pullrequestreview-3035642619)
Concept ACK, thanks for the cleanup.
I personally would prefer doing every single `LogPrintf` -> `LogInfo` inlining here in a scripted diff (so we can get rid of the alias), with follow-up commits fixing the line endings and formatting and stuff.
And I agree that the warn/error split should be done in a separate PR where we can focus on the context (we could probably do a scripted diff for most of the lines that contain warning/error to cover most of those as well)
(https://github.com/bitcoin/bitcoin/pull/32967#pullrequestreview-3035642619)
Concept ACK, thanks for the cleanup.
I personally would prefer doing every single `LogPrintf` -> `LogInfo` inlining here in a scripted diff (so we can get rid of the alias), with follow-up commits fixing the line endings and formatting and stuff.
And I agree that the warn/error split should be done in a separate PR where we can focus on the context (we could probably do a scripted diff for most of the lines that contain warning/error to cover most of those as well)
π¬ l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217522134)
Would it be possible to separate the `LogPrintf` inlining from the `__func__` and rewording changes into focused commits?
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217522134)
Would it be possible to separate the `LogPrintf` inlining from the `__func__` and rewording changes into focused commits?
π¬ l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217527057)
I personally would find it simpler if we inlined every single `LogPrintf` call here, removed the [alias](https://github.com/bitcoin/bitcoin/blob/master/src/logging.h#L369-L370) and the [dedicated test](https://github.com/bitcoin/bitcoin/blob/master/src/test/logging_tests.cpp#L131), and do the info/warn/error differentiation in a separate PR.
I know that means we'd be touching the same lines multiple times, but in that case this PR could simply be a scripted diff with alias removal (+ line endin
...
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217527057)
I personally would find it simpler if we inlined every single `LogPrintf` call here, removed the [alias](https://github.com/bitcoin/bitcoin/blob/master/src/logging.h#L369-L370) and the [dedicated test](https://github.com/bitcoin/bitcoin/blob/master/src/test/logging_tests.cpp#L131), and do the info/warn/error differentiation in a separate PR.
I know that means we'd be touching the same lines multiple times, but in that case this PR could simply be a scripted diff with alias removal (+ line endin
...