💬 achow101 commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1612048492)
ACK d4fb58ae8ae9772d025ead184ef8f2c0ea50df3e

Much simpler to understand, and the more complicated stuff matches up with the descriptions in WIkipedia.

🚀 achow101 merged a pull request: "Introduce secp256k1 module with field and group classes to test framework"
(https://github.com/bitcoin/bitcoin/pull/26222)

💬 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
...

🤔 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)

💬 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

💬 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)
```

💬 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.

🤔 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
...

🤔 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

💬 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

💬 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
...

💬 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).

💬 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.

💬 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.
>
> -----------------------------
...

💬 LarryRuane commented on pull request "logging: use bitset for categories":
(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.

💬 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
...

💬 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
...

💬 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.

✅ MarcoFalke closed a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941)