💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-1523392935)
Push to 60cd07b781bfe46262a5d66c97e257e0dd378f5c:
- rebase on master
- add PR release note
- change how we detect "old" directory use from looking for `blocks/` to looking for `chainstate/`

💬 MarcoFalke commented on issue "CI failure "buster-backports InRelease: The following signatures couldn't be verified because the public key is not available"":
(https://github.com/bitcoin/bitcoin/issues/27531#issuecomment-1523395787)
Yeah, in Cirrus there is a setting to merge the cirrus.yaml of the pull request with the one in the target branch. It is enabled, however it doesn't merge the git contents. Thus, there is a workaround step to achieve that: https://github.com/bitcoin/bitcoin/blob/91ccb62faab21b2b52b089cc04f3a5c1bf6989cc/.cirrus.yml#L27

However, the workaround does not work for Cirrus Docker Env files. Ideally this is fixed by Cirrus CI. I don't see another workaround than either rebasing or removing the Env fe
...

💬 willcl-ark 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-1523439189)
Will this issue be closed by https://github.com/bitcoin/bitcoin/pull/27302 ?

💬 MarcoFalke commented on pull request "Remove now-unnecessary poll, fcntl includes from net(base).cpp":
(https://github.com/bitcoin/bitcoin/pull/27530#issuecomment-1523481640)
Can you add this to iwyu CI to double check and aid reviewers?

💬 MarcoFalke commented on pull request "test: add coverage for rpc error when trying to rescan beyond pruned data":
(https://github.com/bitcoin/bitcoin/pull/25937#issuecomment-1523489631)
lgtm ACK cca4f82b828669ae23f6ac64fb83e068b81ae189

See also https://marcofalke.github.io/b-c-cov/total.coverage/src/wallet/rpc/transactions.cpp.gcov.html

🤔 brunoerg reviewed a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511#pullrequestreview-1402102237)
Concept ACK

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