💬 mzumsande commented on pull request "p2p: Restrict self-advertisements with privacy networks to avoid fingerprinting":
(https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1612067010)
> > Note that this affects only our own self-advertisements, the rules to forward other people's addrs are not changed.
>
> Would this make our own addr stand out too? Or could we still forward our own addr by coincidence outside of self-advertising?
Yes, we could still forward it: When we receive a gossipped address we choose 1-2 random peers (that stay the same for 24 hours, see [RelayAddress](https://github.com/bitcoin/bitcoin/blob/7952a5934a1e071ca24a51483d058174e7b6de43/src/net_proces
...
  (https://github.com/bitcoin/bitcoin/pull/27411#issuecomment-1612067010)
> > Note that this affects only our own self-advertisements, the rules to forward other people's addrs are not changed.
>
> Would this make our own addr stand out too? Or could we still forward our own addr by coincidence outside of self-advertising?
Yes, we could still forward it: When we receive a gossipped address we choose 1-2 random peers (that stay the same for 24 hours, see [RelayAddress](https://github.com/bitcoin/bitcoin/blob/7952a5934a1e071ca24a51483d058174e7b6de43/src/net_proces
...
🤔 jonatack reviewed a pull request: "test: Use same timeout for all index sync"
(https://github.com/bitcoin/bitcoin/pull/27988#pullrequestreview-1503945589)
Approach ACK
(review beg for #27425 doing a ~similar test util extraction change)
  (https://github.com/bitcoin/bitcoin/pull/27988#pullrequestreview-1503945589)
Approach ACK
(review beg for #27425 doing a ~similar test util extraction change)
💬 jonatack commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245722202)
Perhaps call it `WaitUntilIndexSynced`
and `s/Block/Wait/`, or drop the Doxygen comment if the naming is clear enough
  (https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245722202)
Perhaps call it `WaitUntilIndexSynced`
and `s/Block/Wait/`, or drop the Doxygen comment if the naming is clear enough
💬 jonatack commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245731192)
Here and in the function declaration
```suggestion
void IndexWaitSynced(const BaseIndex& index)
```
  (https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1245731192)
Here and in the function declaration
```suggestion
void IndexWaitSynced(const BaseIndex& index)
```
💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1245800503)
Cool thanks!. Pushed it with few tiny differences because I think that, at least for now, we don't need the indirections introduced by making `GetFirstStoredBlock` a `FindFirstStored` wrapper.
  (https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1245800503)
Cool thanks!. Pushed it with few tiny differences because I think that, at least for now, we don't need the indirections introduced by making `GetFirstStoredBlock` a `FindFirstStored` wrapper.
🤔 ismaelsadeeq reviewed a pull request: "assumeutxo (2)"
(https://github.com/bitcoin/bitcoin/pull/27596#pullrequestreview-1504152824)
While testing.
>Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk
`FlushSnapshotToDisk: saving snapshot chain state (10588 MB) started` takes about 5 hours to complete on pop-OS 22.
During the `FlushSnapshotToDisk` process, I believe there should be some progress indication, similar to what occurs during the snapshot loading process. Currently, it appears to be like it is stuck.
Shutting down `bit
...
  (https://github.com/bitcoin/bitcoin/pull/27596#pullrequestreview-1504152824)
While testing.
>Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk
`FlushSnapshotToDisk: saving snapshot chain state (10588 MB) started` takes about 5 hours to complete on pop-OS 22.
During the `FlushSnapshotToDisk` process, I believe there should be some progress indication, similar to what occurs during the snapshot loading process. Currently, it appears to be like it is stuck.
Shutting down `bit
...
🤔 jonatack reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1504151388)
Initial concept/approach ACK 500e31e87b33418a0747a0f3118db9ceb23896be
  (https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1504151388)
Initial concept/approach ACK 500e31e87b33418a0747a0f3118db9ceb23896be
💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1245804422)
In 930b1c7323be84ad7fb8371ee744f201325f18bb agree with @MarcoFalke to put this next to `nTime`, and it may be nice to document how each is used and why the need for both
  (https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1245804422)
In 930b1c7323be84ad7fb8371ee744f201325f18bb agree with @MarcoFalke to put this next to `nTime`, and it may be nice to document how each is used and why the need for both
💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1245801404)
In e16fb7d212bf1dbb5086ed8f452e8e80acbe8325 consider adding the `info_for_relay()` getter declaration next to `info()` and `infoAll()`
<details><summary>diff</summary><p>
```diff
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -411,8 +411,6 @@ public:
using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;
std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order
 
- TxMempoolInfo in
...
  (https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1245801404)
In e16fb7d212bf1dbb5086ed8f452e8e80acbe8325 consider adding the `info_for_relay()` getter declaration next to `info()` and `infoAll()`
<details><summary>diff</summary><p>
```diff
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -411,8 +411,6 @@ public:
using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;
std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order
- TxMempoolInfo in
...
💬 mzumsande commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1612168354)
> Looks like there is a variation with ERROR: Commit: Failed to commit latest basic block filter index state?
>And the same without the error:
It's different tests for different indexes: `txindex_tests/txindex_initial_sync` fails without the Commit Error, `blockfilter_index_tests/blockfilter_index_initial_sync` fails with it (but same root cause obviously).
  (https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1612168354)
> Looks like there is a variation with ERROR: Commit: Failed to commit latest basic block filter index state?
>And the same without the error:
It's different tests for different indexes: `txindex_tests/txindex_initial_sync` fails without the Commit Error, `blockfilter_index_tests/blockfilter_index_initial_sync` fails with it (but same root cause obviously).
💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1612198266)
Found an issue with prefix cursor handling, updated the tests to check for that. Also pulled in @TheCharlatan's fuzz test and changed several asserts to instead throw exceptions which resolves the fuzz crash.
  (https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-1612198266)
Found an issue with prefix cursor handling, updated the tests to check for that. Also pulled in @TheCharlatan's fuzz test and changed several asserts to instead throw exceptions which resolves the fuzz crash.
💬 MarcusMWilliams commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1612199588)
Marcus Maurice Williams
On Mon, May 8, 2023, 10:12 AM jamesob ***@***.***> wrote:
>
> - Background and FAQ:
> https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
> - Prior progress/project:
> https://github.com/bitcoin/bitcoin/projects/11
> - Replaces #15606 <https://github.com/bitcoin/bitcoin/pull/15606>,
> which was closed due to Github slowness. Original description and
> commentary can be found there.
>
> -----------------------------
...
  (https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1612199588)
Marcus Maurice Williams
On Mon, May 8, 2023, 10:12 AM jamesob ***@***.***> wrote:
>
> - Background and FAQ:
> https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
> - Prior progress/project:
> https://github.com/bitcoin/bitcoin/projects/11
> - Replaces #15606 <https://github.com/bitcoin/bitcoin/pull/15606>,
> which was closed due to Github slowness. Original description and
> commentary can be found there.
>
> -----------------------------
...
💬 LarryRuane commented on pull request "logging: use bitset for categories":
(https://github.com/bitcoin/bitcoin/pull/26697#issuecomment-1612227295)
Rebased to fix merge conflict
  (https://github.com/bitcoin/bitcoin/pull/26697#issuecomment-1612227295)
Rebased to fix merge conflict
💬 Trrvrr commented on pull request "[24.1] Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27660#issuecomment-1612298568)
> Final changes for `v24.1`.
>
> PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
  (https://github.com/bitcoin/bitcoin/pull/27660#issuecomment-1612298568)
> Final changes for `v24.1`.
>
> PR for bitcoincore.org is here: https://github.com/bitcoin-core/bitcoincore.org/pull/968.
💬 craigraw commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612482503)
To be clear, my desire for an efficient way to retrieve fee rate histogram data has nothing to do with Bitcoin Core's fee estimation algorithm. Because fee estimation is inherently difficult and no algorithm can know the user's time preference for every transaction, Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time pre
...
  (https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612482503)
To be clear, my desire for an efficient way to retrieve fee rate histogram data has nothing to do with Bitcoin Core's fee estimation algorithm. Because fee estimation is inherently difficult and no algorithm can know the user's time preference for every transaction, Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time pre
...
💬 MarcoFalke commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612498668)
> One workaround is to use `getrawmempool false` followed by repeated calls to `getmempoolentry`. This of course is even more inefficient, leading any reasonable client implementation to maintain a replica of mempool entries to avoid having to load them all, one by one, every time a fee rate histogram is required. This trades better performance for significantly increased memory utilisation.
I think the `getmempoolentry` can be batched for a slight improvement. Also, you don't have to store t
...
  (https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612498668)
> One workaround is to use `getrawmempool false` followed by repeated calls to `getmempoolentry`. This of course is even more inefficient, leading any reasonable client implementation to maintain a replica of mempool entries to avoid having to load them all, one by one, every time a fee rate histogram is required. This trades better performance for significantly increased memory utilisation.
I think the `getmempoolentry` can be batched for a slight improvement. Also, you don't have to store t
...
💬 MarcoFalke commented on pull request "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py":
(https://github.com/bitcoin/bitcoin/pull/27941#issuecomment-1612502537)
> The longpoll request is supposed to return immediately if the conditions have been met. The token includes a reference to the transaction update state as a counter.
Thanks, I'll take another look later.
  (https://github.com/bitcoin/bitcoin/pull/27941#issuecomment-1612502537)
> The longpoll request is supposed to return immediately if the conditions have been met. The token includes a reference to the transaction update state as a counter.
Thanks, I'll take another look later.
✅ MarcoFalke closed a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941)
  (https://github.com/bitcoin/bitcoin/pull/27941)
💬 MarcoFalke commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#issuecomment-1612507797)
Are you sure you rebased on *current* master?
  (https://github.com/bitcoin/bitcoin/pull/24005#issuecomment-1612507797)
Are you sure you rebased on *current* master?
💬 MarcoFalke commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057)
CI fuzz:
```
terminate called after throwing an instance of 'NonFatalCheckError'
what(): Internal bug detected: "parser.m_keys.size() != 0"
script/descriptor.cpp:1808 (ParseScript)
Bitcoin Core v25.99.0-501b80414422
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
==16697== ERROR: libFuzzer: deadly signal
  (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057)
CI fuzz:
```
terminate called after throwing an instance of 'NonFatalCheckError'
what(): Internal bug detected: "parser.m_keys.size() != 0"
script/descriptor.cpp:1808 (ParseScript)
Bitcoin Core v25.99.0-501b80414422
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
==16697== ERROR: libFuzzer: deadly signal