💬 vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2344157017)
Looks like, according to `git range-diff b7ff6a611a...3bdff9a154`, this and the suggestion above were reverted in the latest push? If that was not intentional, then maybe amend this change in the latest commit:
```diff
--- i/test/functional/p2p_ibd_stalling.py
+++ w/test/functional/p2p_ibd_stalling.py
@@ -63,29 +63,30 @@ class P2PIBDStallingTest(BitcoinTestFramework):
blocks.append(create_block(tip, create_coinbase(height), block_time))
blocks[-1].solve()
...
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2344157017)
Looks like, according to `git range-diff b7ff6a611a...3bdff9a154`, this and the suggestion above were reverted in the latest push? If that was not intentional, then maybe amend this change in the latest commit:
```diff
--- i/test/functional/p2p_ibd_stalling.py
+++ w/test/functional/p2p_ibd_stalling.py
@@ -63,29 +63,30 @@ class P2PIBDStallingTest(BitcoinTestFramework):
blocks.append(create_block(tip, create_coinbase(height), block_time))
blocks[-1].solve()
...
⚠️ Raimo33 opened an issue: "bench: unrealistic ConnectBlock benchmarks"
(https://github.com/bitcoin/bitcoin/issues/33375)
The current `ConnectBlock` benchmarks in `bench/connectblock.cpp` do not reflect realistic mainnet workloads due to three key issues:
#### 1. Unrealistic block composition
Every benchmarked block is constructed with a highly artificial transaction pattern:
```c
/*
* - Each transaction has the same number of inputs and outputs
* - All Taproot inputs use simple key path spends (no script path spends)
* - All signatures use SIGHASH_ALL (default sighash)
* - Each transaction spends all output
...
(https://github.com/bitcoin/bitcoin/issues/33375)
The current `ConnectBlock` benchmarks in `bench/connectblock.cpp` do not reflect realistic mainnet workloads due to three key issues:
#### 1. Unrealistic block composition
Every benchmarked block is constructed with a highly artificial transaction pattern:
```c
/*
* - Each transaction has the same number of inputs and outputs
* - All Taproot inputs use simple key path spends (no script path spends)
* - All signatures use SIGHASH_ALL (default sighash)
* - Each transaction spends all output
...
💬 vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3285189452)
ACK 3bdff9a154733f8f9f379448f5595a2e90474bc6 but see https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2344157017, non-blocking IMO, would be nice to address
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3285189452)
ACK 3bdff9a154733f8f9f379448f5595a2e90474bc6 but see https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2344157017, non-blocking IMO, would be nice to address
🤔 stickies-v reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3216090598)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3216090598)
Concept ACK
💬 stickies-v commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2343838851)
nit: using `std::numeric_limits<size_t>::max()` would avoid using a macro, and has my preference
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2343838851)
nit: using `std::numeric_limits<size_t>::max()` would avoid using a macro, and has my preference
💬 stickies-v commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344204033)
To make this more visible than just the log line, you can use `node::warnings` to expose it through RPC (e.g. `getnetworkinfo["warnings"]` and GUI:
<details>
<summary>git diff on 080a21cb12</summary>
```diff
diff --git a/src/init.cpp b/src/init.cpp
index caf1515804..782cb7f6c4 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -56,6 +56,7 @@
#include <node/mempool_persist.h>
#include <node/mempool_persist_args.h>
#include <node/miner.h>
+#include <node/warnings.h>
#include <nod
...
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344204033)
To make this more visible than just the log line, you can use `node::warnings` to expose it through RPC (e.g. `getnetworkinfo["warnings"]` and GUI:
<details>
<summary>git diff on 080a21cb12</summary>
```diff
diff --git a/src/init.cpp b/src/init.cpp
index caf1515804..782cb7f6c4 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -56,6 +56,7 @@
#include <node/mempool_persist.h>
#include <node/mempool_persist_args.h>
#include <node/miner.h>
+#include <node/warnings.h>
#include <nod
...
💬 vasild commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2344214394)
What can be removed? The `noexcept` or the entire destructor?
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2344214394)
What can be removed? The `noexcept` or the entire destructor?
📝 Raimo33 opened a pull request: "[WIP] cache: remove redundant find() call"
(https://github.com/bitcoin/bitcoin/pull/33376)
## Summary
This PR halves the number of implicit calls to `::find()` by using an optimistic approach: insert preemptively, remove O(1) if already spent.
## Motivation
Currently, the `::find()` calls that originate from `CCoinsViewCache::BatchWrite()` account for ~2.9% of total IBD time as shown by this flamegraph from issue #32832

## Benchmarks
Benchmarks can't currently measure this
...
(https://github.com/bitcoin/bitcoin/pull/33376)
## Summary
This PR halves the number of implicit calls to `::find()` by using an optimistic approach: insert preemptively, remove O(1) if already spent.
## Motivation
Currently, the `::find()` calls that originate from `CCoinsViewCache::BatchWrite()` account for ~2.9% of total IBD time as shown by this flamegraph from issue #32832

## Benchmarks
Benchmarks can't currently measure this
...
📝 justinjsd opened a pull request: "new gen"
(https://github.com/bitcoin/bitcoin/pull/33377)
<!--
*** 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 improve coverage are a
...
(https://github.com/bitcoin/bitcoin/pull/33377)
<!--
*** 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 improve coverage are a
...
✅ fanquake closed a pull request: "new gen"
(https://github.com/bitcoin/bitcoin/pull/33377)
(https://github.com/bitcoin/bitcoin/pull/33377)
💬 instagibbs commented on pull request "txgraph: randomize order of same-feerate distinct-cluster transactions":
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2344302600)
I didn't really mean to dive into the exact numbers, but yeah, for "rare collisions".
(https://github.com/bitcoin/bitcoin/pull/33335#discussion_r2344302600)
I didn't really mean to dive into the exact numbers, but yeah, for "rare collisions".
✅ fanquake closed an issue: "Zero output not cleared"
(https://github.com/bitcoin/bitcoin/issues/33265)
(https://github.com/bitcoin/bitcoin/issues/33265)
🚀 fanquake merged a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268)
(https://github.com/bitcoin/bitcoin/pull/33268)
💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2344324168)
> Loading a large file
This is run on request, before anything else loads, it's not *that* large, only 160 MB memory is needed.
For reference, applying the mentioned `dbcache=4` (which isn't used here yet) still makes the node use > 1 GB memory:
<img width="1202" height="631" alt="image" src="https://github.com/user-attachments/assets/1867ba38-ba35-4ce5-86de-5bfe8add145b" />
> Here's roughly what I'm thinking: [ajtowns/bitcoin@202509-reobfus (commits)](https://github.com/ajtowns/bitco
...
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2344324168)
> Loading a large file
This is run on request, before anything else loads, it's not *that* large, only 160 MB memory is needed.
For reference, applying the mentioned `dbcache=4` (which isn't used here yet) still makes the node use > 1 GB memory:
<img width="1202" height="631" alt="image" src="https://github.com/user-attachments/assets/1867ba38-ba35-4ce5-86de-5bfe8add145b" />
> Here's roughly what I'm thinking: [ajtowns/bitcoin@202509-reobfus (commits)](https://github.com/ajtowns/bitco
...
👍 instagibbs approved a pull request: "txgraph: randomize order of same-feerate distinct-cluster transactions"
(https://github.com/bitcoin/bitcoin/pull/33335#pullrequestreview-3216844701)
ACK 593d418137e4802bbe229707dcda5796522e2e5e
double-checked Trim benchmark just in case, looks unchanged
(https://github.com/bitcoin/bitcoin/pull/33335#pullrequestreview-3216844701)
ACK 593d418137e4802bbe229707dcda5796522e2e5e
double-checked Trim benchmark just in case, looks unchanged
💬 fanquake commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3285414401)
Backported to `30.x` in #33356.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3285414401)
Backported to `30.x` in #33356.
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344380721)
I was deliberately trying to avoid that, I find that completely unreadable
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344380721)
I was deliberately trying to avoid that, I find that completely unreadable
💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344386717)
Good idea, thanks. Do you think we should also add the reason for the enable/disable (we'd store an enum instead of the optional bool which would provide more info on *why* signature verification was enabled or disabled)?
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2344386717)
Good idea, thanks. Do you think we should also add the reason for the enable/disable (we'd store an enum instead of the optional bool which would provide more info on *why* signature verification was enabled or disabled)?
💬 fanquake commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3285583699)
Backported to `29.x` in #33344.
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3285583699)
Backported to `29.x` in #33344.
💬 purpleKarrot commented on pull request "[WIP] cache: remove redundant find() call":
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344612738)
Can this condition be checked before the insert?
(https://github.com/bitcoin/bitcoin/pull/33376#discussion_r2344612738)
Can this condition be checked before the insert?