💬 achow101 commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2284815826)
ACK 204ca67bba263018374fe86d7a6867362d09536f
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2284815826)
ACK 204ca67bba263018374fe86d7a6867362d09536f
🚀 achow101 merged a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209)
(https://github.com/bitcoin/bitcoin/pull/28209)
💬 achow101 commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2284832694)
ACK 5215c925d1382e71c9e1d642fced8a152c629c7f
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2284832694)
ACK 5215c925d1382e71c9e1d642fced8a152c629c7f
🚀 achow101 merged a pull request: "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin"
(https://github.com/bitcoin/bitcoin/pull/30326)
(https://github.com/bitcoin/bitcoin/pull/30326)
🚀 achow101 merged a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246)
(https://github.com/bitcoin/bitcoin/pull/30246)
📝 paplorinc opened a pull request: "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix"
(https://github.com/bitcoin/bitcoin/pull/30641)
Related to https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1703187118
WIP: First, I have to make sure I can reproduce the Sonar warning, I'll push the fix after that.
(https://github.com/bitcoin/bitcoin/pull/30641)
Related to https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1703187118
WIP: First, I have to make sure I can reproduce the Sonar warning, I'll push the fix after that.
💬 andrewtoth commented on pull request "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix":
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284885871)
@paplorinc you can mark this as a draft until you are ready to have it reviewed.
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284885871)
@paplorinc you can mark this as a draft until you are ready to have it reviewed.
💬 fanquake commented on pull request "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix":
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284886011)
There's no need to open a PR with a TODO comment, for an issue which you haven't yet reproduced, for a tool we don't actively use.
Feel free re-open when you have an actual change to suggest.
Also, you can open draft PRs to indicate when changes are not ready for review.
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284886011)
There's no need to open a PR with a TODO comment, for an issue which you haven't yet reproduced, for a tool we don't actively use.
Feel free re-open when you have an actual change to suggest.
Also, you can open draft PRs to indicate when changes are not ready for review.
✅ fanquake closed a pull request: "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix"
(https://github.com/bitcoin/bitcoin/pull/30641)
(https://github.com/bitcoin/bitcoin/pull/30641)
💬 paplorinc commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284890283)
> Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
I assumed (without diving into the code) that this meant an optimization.
<details>
<summary>Did 2x5 reindex checks until 500k blocks - 0% change.</summary>
Time (mean ± σ): 15396.778 s ± 31.337 s [User: 81183.230 s, System: 364.223 s]
Range (min … max): 15363.334 s … 15427.586 s 5 runs
Time (mean ± σ): 15410.168 s ± 20.832 s [User: 81268.923 s,
...
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284890283)
> Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.
I assumed (without diving into the code) that this meant an optimization.
<details>
<summary>Did 2x5 reindex checks until 500k blocks - 0% change.</summary>
Time (mean ± σ): 15396.778 s ± 31.337 s [User: 81183.230 s, System: 364.223 s]
Range (min … max): 15363.334 s … 15427.586 s 5 runs
Time (mean ± σ): 15410.168 s ± 20.832 s [User: 81268.923 s,
...
💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284891365)
@maflcko:
> This is trivial to find in libFuzzer with a small enough max_len and value profile ( for example, -use_value_profile=1 -max_len=24).
How trivial is trivial? I have a set up that can definitely find it, and that setting is taking quite some time...
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284891365)
@maflcko:
> This is trivial to find in libFuzzer with a small enough max_len and value profile ( for example, -use_value_profile=1 -max_len=24).
How trivial is trivial? I have a set up that can definitely find it, and that setting is taking quite some time...
💬 sipa commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284893423)
@paplorinc Yes, it is an optimization, but only affects the `scantxoutset` and `gettxoutsetinfo` RPCs. Normal synchronization should be unaffected.
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284893423)
@paplorinc Yes, it is an optimization, but only affects the `scantxoutset` and `gettxoutsetinfo` RPCs. Normal synchronization should be unaffected.
💬 mzumsande commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2284894942)
Maybe copying the string into a static or global variable would help rule out a lifetime issue?
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 9126d8e928..683d13e0e5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1863,6 +1863,8 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra
CTxMemPool& pool{*active_chainstate.GetMempool()};
std::vector<COutPoint> coins_to_uncache;
+ static char string_buffer[120];
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2284894942)
Maybe copying the string into a static or global variable would help rule out a lifetime issue?
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 9126d8e928..683d13e0e5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1863,6 +1863,8 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra
CTxMemPool& pool{*active_chainstate.GetMempool()};
std::vector<COutPoint> coins_to_uncache;
+ static char string_buffer[120];
...
📝 theStack opened a pull request: "doc: add missing "testnet4" network string in RPC/init help texts"
(https://github.com/bitcoin/bitcoin/pull/30642)
The following bitcoind parameters / RPC calls still missed the "testnet4" network string:
- `-chain=` parameter
- `getblockchaininfo` RPC, `"chain"` result
- `getmininginfo` RPC, `"chain"` result
The occurences were found via `$ git grep \".*main.*test.*\"`.
(https://github.com/bitcoin/bitcoin/pull/30642)
The following bitcoind parameters / RPC calls still missed the "testnet4" network string:
- `-chain=` parameter
- `getblockchaininfo` RPC, `"chain"` result
- `getmininginfo` RPC, `"chain"` result
The occurences were found via `$ git grep \".*main.*test.*\"`.
📝 paplorinc opened a pull request: "coins: Add move operations to CCoinsCacheEntry"
(https://github.com/bitcoin/bitcoin/pull/30643)
Related to https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1703187118
Added defaulted move constructor and move assignment operator to `CCoinsCacheEntry`.
And to make sure the mentioned Sonar warnings aren't triggered by CI anymore, I've modernized the call-site minimally.
(https://github.com/bitcoin/bitcoin/pull/30643)
Related to https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1703187118
Added defaulted move constructor and move assignment operator to `CCoinsCacheEntry`.
And to make sure the mentioned Sonar warnings aren't triggered by CI anymore, I've modernized the call-site minimally.
💬 paplorinc commented on pull request "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix":
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284920490)
I'm a bit surprised at these reactions, but opened https://github.com/bitcoin/bitcoin/pull/30643 with the proposed fix.
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284920490)
I'm a bit surprised at these reactions, but opened https://github.com/bitcoin/bitcoin/pull/30643 with the proposed fix.
💬 furszy commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380473)
done
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380473)
done
💬 furszy commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380532)
done
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380532)
done
💬 andrewtoth commented on pull request "coins: Add move operations to CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/30643#issuecomment-2284929509)
Don't we also need to add move constructor to `Coin`? Its prevector is the only thing that really benefits from being moved I believe.
(https://github.com/bitcoin/bitcoin/pull/30643#issuecomment-2284929509)
Don't we also need to add move constructor to `Coin`? Its prevector is the only thing that really benefits from being moved I believe.
🤔 stickies-v reviewed a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071#pullrequestreview-2232822586)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/29071#pullrequestreview-2232822586)
Approach ACK