π TheCharlatan approved a pull request: "init: [gui] Avoid UB/crash in InitAndLoadChainstate"
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3035308818)
ACK fac90e5261b811739ada56e06ea793a12f9c2c3d
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3035308818)
ACK fac90e5261b811739ada56e06ea793a12f9c2c3d
π¬ Eunovo commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2217261456)
`ToPrivateString` will be called on a non-watch-only wallet when the user tries RPC `listdescriptors[private=true]`. In this case, `has_priv_key` will be `false` and the RPC call should fail as expected.
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2217261456)
`ToPrivateString` will be called on a non-watch-only wallet when the user tries RPC `listdescriptors[private=true]`. In this case, `has_priv_key` will be `false` and the RPC call should fail as expected.
β οΈ fanquake opened an issue: "intermittent timeout in wallet_signer.py : sendall timed out"
(https://github.com/bitcoin/bitcoin/issues/33015)
https://cirrus-ci.com/task/6466141373071360?logs=ci#L1714:
```bash
[17:24:42.699] node1 2025-07-18T21:24:35.153536Z [scheduler] [net.cpp:2387] [void CConnman::DumpAddresses()] [net] Flushed 0 addresses to peers.dat 0ms
[17:24:42.699] node0 2025-07-18T21:24:35.182594Z [scheduler] [net.cpp:2387] [void CConnman::DumpAddresses()] [net] Flushed 0 addresses to peers.dat 1ms
[17:24:42.699] test 2025-07-18T21:24:41.997879Z TestFramework (ERROR): Called Process failed with 'error: timeout on tran
...
(https://github.com/bitcoin/bitcoin/issues/33015)
https://cirrus-ci.com/task/6466141373071360?logs=ci#L1714:
```bash
[17:24:42.699] node1 2025-07-18T21:24:35.153536Z [scheduler] [net.cpp:2387] [void CConnman::DumpAddresses()] [net] Flushed 0 addresses to peers.dat 0ms
[17:24:42.699] node0 2025-07-18T21:24:35.182594Z [scheduler] [net.cpp:2387] [void CConnman::DumpAddresses()] [net] Flushed 0 addresses to peers.dat 1ms
[17:24:42.699] test 2025-07-18T21:24:41.997879Z TestFramework (ERROR): Called Process failed with 'error: timeout on tran
...
π¬ DerEwige commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092301624)
What about rpc calls that currently only alow minimum value of 1 sat/vb?
eg: https://developer.bitcoin.org/reference/rpc/bumpfee.html
Should those endpoints be changed at the same time as the min relay fee?
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092301624)
What about rpc calls that currently only alow minimum value of 1 sat/vb?
eg: https://developer.bitcoin.org/reference/rpc/bumpfee.html
Should those endpoints be changed at the same time as the min relay fee?
π bigshiny90 opened a pull request: "test: Add functional tests for blockreconstructionextratxn"
(https://github.com/bitcoin/bitcoin/pull/33016)
This adds tests for the `-blockreconstructionextratxn` parameter which controls the extra transaction pool used for compact block reconstruction.
Uses RBF transaction pairs to populate the pool since that's a straightforward way to get transactions into the extra pool - send an original, then replace it with higher fee, and the original ends up in the extra pool.
Note: Targeting 29.x because the extra transaction pool eviction behavior tested here doesn't currently work on master.
(https://github.com/bitcoin/bitcoin/pull/33016)
This adds tests for the `-blockreconstructionextratxn` parameter which controls the extra transaction pool used for compact block reconstruction.
Uses RBF transaction pairs to populate the pool since that's a straightforward way to get transactions into the extra pool - send an original, then replace it with higher fee, and the original ends up in the extra pool.
Note: Targeting 29.x because the extra transaction pool eviction behavior tested here doesn't currently work on master.
π€ Marodero56 reviewed a pull request: "test: Add functional tests for blockreconstructionextratxn"
(https://github.com/bitcoin/bitcoin/pull/33016#pullrequestreview-3035442509)
Hello
(https://github.com/bitcoin/bitcoin/pull/33016#pullrequestreview-3035442509)
Hello
π€ Marodero56 reviewed a pull request: "test: Add functional tests for blockreconstructionextratxn"
(https://github.com/bitcoin/bitcoin/pull/33016#pullrequestreview-3035442705)
Hello π
(https://github.com/bitcoin/bitcoin/pull/33016#pullrequestreview-3035442705)
Hello π
π¬ kurapika007 commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092464118)
> In general, low-fee transactions should be batched, and pushed on-chain as high-fee transactions. Which means, that 10 users, paying below 1 sat/vB, can consolidate their coins, and make a transaction, which would appear as at least 1 sat/vB on-chain. When we have 1 sat/vB, then miners are guaranteed to get at least 0.01 BTC in fees per block, as long as blocks are full, and users are willing to transact. By lowering that limit, it may cause problems, when next halvings will lower the basic
...
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3092464118)
> In general, low-fee transactions should be batched, and pushed on-chain as high-fee transactions. Which means, that 10 users, paying below 1 sat/vB, can consolidate their coins, and make a transaction, which would appear as at least 1 sat/vB on-chain. When we have 1 sat/vB, then miners are guaranteed to get at least 0.01 BTC in fees per block, as long as blocks are full, and users are willing to transact. By lowering that limit, it may cause problems, when next halvings will lower the basic
...
π¬ romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3092464779)
I have split the content of this PR into 2 commits:
- 784459ac79 is adding the new `/rest/txfromblock/` endpoint
- 1b928f58fc is adding an optional `locationsindex` for optimizing lookups
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3092464779)
I have split the content of this PR into 2 commits:
- 784459ac79 is adding the new `/rest/txfromblock/` endpoint
- 1b928f58fc is adding an optional `locationsindex` for optimizing lookups
π¬ 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-3092466357)
I think, the main problem is in the IBD detection logic in ChainstateManager::IsInitialBlockDownload()
https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2035
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.
I think this can be solved by modifying IBD detection to consider both tip age AND block distance.
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092466357)
I think, the main problem is in the IBD detection logic in ChainstateManager::IsInitialBlockDownload()
https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2035
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.
I think this can be solved by modifying IBD detection to consider both tip age AND block distance.
π¬ sipa 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-3092471548)
> 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.
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3092471548)
> 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.
π¬ 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:
...