💬 MarcoFalke commented on pull request "script: remove unused bitwise `CScriptNum` operators":
(https://github.com/bitcoin/bitcoin/pull/27096#issuecomment-1523501332)
Idk, this is probably too hard to review to be worth it. You are removing an overload, so reviewers need to check how this interacts with overload resolution. Maybe just close and reopen/mention it the next time the code is touched an reviewers are primed on it?

💬 MarcoFalke commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1523503942)
Maybe even with gcc 13.1, now that it is about to be released?

💬 MarcoFalke commented on pull request "Safegcd-based modular inverses in MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/21590#issuecomment-1523507720)
On Windows CI this will eat the bench CPU without terminating?

💬 MarcoFalke commented on pull request "test: test banlist database recreation":
(https://github.com/bitcoin/bitcoin/pull/26794#issuecomment-1523512721)
lgtm ACK 4bdcf571584cbe44ad5533a3c991d3b0b4b2c84f

💬 MarcoFalke commented on pull request "Move `CopyrightHolders()` and `LicenseInfo()` into `libbitcoin_common`":
(https://github.com/bitcoin/bitcoin/pull/26688#issuecomment-1523517315)
I think this is a bugfix/requirement for https://github.com/bitcoin/bitcoin/pull/26504, so I am not sure why it was closed but the other not?

💬 ryanofsky commented on issue "Relative paths named in the -conf parameter reset when parsing datadir in named config":
(https://github.com/bitcoin/bitcoin/issues/19990#issuecomment-1523524847)
> Will this issue be closed by #27302 ?

Yes specifically it is fixed by the third commit 0319de5cbedd1a8f8766cfec61151c58b3fb27ef from #27302. That commit changes the `ArgsManager::GetConfigFilePath` definition so the "The specified config file %s does not exist" warning is no longer triggered:

https://github.com/bitcoin/bitcoin/blob/91ccb62faab21b2b52b089cc04f3a5c1bf6989cc/src/init/common.cpp#L125-L130

👍 brunoerg approved a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511#pullrequestreview-1402136223)
tACK c505611f08a850acb97dea9cc03b36aa468929ca

just tested it with my "clearnet only" node, results:
```sh
➜ bitcoin-core-dev git:(27511-stratospher) ✗ ./src/bitcoin-cli getaddrmaninfo
{
"ipv4": {
"new": 28765,
"tried": 277,
"total": 29042
},
"ipv6": {
"new": 7245,
"tried": 67,
"total": 7312
},
"onion": {
"new": 0,
"tried": 0,
"total": 0
},
"i2p": {
"new": 0,
"tried": 0,
"total": 0
},
"cjdns": {
"ne
...

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1177996808)
I'm going to revert this so it can be fixed in https://github.com/bitcoin/bitcoin/pull/27191

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1523589827)
Ok I updated the unit test: removed the time modified checks because all we need to check is if the content of the file has been changed, which it is NOT if `dbp != nullptr`. The read-only check in the functional test covers the rest.

💬 svanstaa commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1523843886)
Why is `use_current_time` defaulting to true? As far as i understand, this would set all the broadcast times of the transactions in mempool.dat to the current time, effectively extending their lifetimes, to the point that (time-) expired txns would be dumped into the network again.
So I would suggest defaulting the value to false, unless there is some rationale behind it that is not obvious.

💬 michaelfolkson commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1523860147)
Also while we're sneaking [PR review club](https://bitcoincore.reviews/27460) questions on here what do people here use this for? I'm assuming it is useful for mempool dev work (specific details would be interesting) but I can't imagine it is common for a user to want this.

Concept ACK (the RPC approach seems superior assuming people are currently importing mempools without a RPC being available)

🤔 George43566 reviewed a pull request: "Remove now-unnecessary poll, fcntl includes from net(base).cpp"
(https://github.com/bitcoin/bitcoin/pull/27530#pullrequestreview-1402612799)
Okay

⚠️ ilbarillo2014 opened an issue: "Bitcoin Core v22.0.0 crashes while syncronizing the first local wallet transaction"
(https://github.com/bitcoin/bitcoin/issues/27533)
Hi all,
I'm running Bitcoin Core v22.0.0 and I'm syncronizing all the full blocks.
When it arrives exactly at processing the transaction when I received on 3-01-2018 my first small amount of bitcoin in my wallet, it crashes.

In debug.log I can see only this last message:
2023-04-26T19:01:29Z UpdateTip: new best=000000000000000000667c6320fa787dbd80768f3a9babcf7d7b5b36956f918b height=502410 version=0x20000000 log2_work=87.792928 tx=288732313 date='2018-01-03T19:34:24Z' progress=0.364601 cach
...

📝 satsie opened a pull request: "rpc: add 'getnetmsgstats', new rpc to view network message statistics"
(https://github.com/bitcoin/bitcoin/pull/27534)
Introduce a new RPC, `getnetmsgstats` to retrieve network message statistics. This work addresses https://github.com/bitcoin/bitcoin/issues/26337. More information on the RPC design and implementation can be found in that issue.

**_Massive_** thank-you to @amitiuttarwar, @vasild, and @ajtowns for their help on this :pray: Over the course of several months, they have patiently provided a tremendous amount of guidance and assistance in more ways than I can count!

-------

## getnetmsgstat
...

📝 satsie converted_to_draft a pull request: "rpc: add 'getnetmsgstats', new rpc to view network message statistics"
(https://github.com/bitcoin/bitcoin/pull/27534)
Introduce a new RPC, `getnetmsgstats` to retrieve network message statistics. This work addresses https://github.com/bitcoin/bitcoin/issues/26337. More information on the RPC design and implementation can be found in that issue.

**_Massive_** thank-you to @amitiuttarwar, @vasild, and @ajtowns for their help on this :pray: Over the course of several months, they have patiently provided a tremendous amount of guidance and assistance in more ways than I can count!

-------

## getnetmsgstat
...

💬 petertodd commented on issue "add ability to remove (dust) UTXOs":
(https://github.com/bitcoin/bitcoin/issues/23875#issuecomment-1524286578)
@rebroad The economic way to deal with dust UTXO's would be to soft-fork in a per-UTXO storage "tax", that eventually makes a UTXO unsepndable. At that point it can be removed from the UTXO set. You can't actually recover that tax in that circumstance with a soft-fork. But that's ok - the amount of money tied up in dust UTXO's isn't very big anyway.

💬 petertodd commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1524311380)
@Sjors Added a test.

🚀 fanquake merged a pull request: "test: add coverage for rpc error when trying to rescan beyond pruned data"
(https://github.com/bitcoin/bitcoin/pull/25937)

✅ hebasto closed a pull request: "Add `UNREACHABLE` macro and drop `-Wreturn-type`/`C4715` warnings suppressions"
(https://github.com/bitcoin/bitcoin/pull/26504)

💬 hebasto commented on pull request "Move `CopyrightHolders()` and `LicenseInfo()` into `libbitcoin_common`":
(https://github.com/bitcoin/bitcoin/pull/26688#issuecomment-1524812974)
> I think this is a bugfix/requirement for https://github.com/bitcoin/bitcoin/pull/26504, so I am not sure why it was closed but the other not?

It is closed as well now.