💬 turkycat commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1167679089)
nit: use of assignment operator on line 126 while using initializer syntax for the same type on 123.

```suggestion
FlatFilePos pos2{blockman.SaveBlockToDisk(block2, /*nHeight=*2, chain, *params, /*dbp=*/nullptr)};
```

💬 turkycat commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1167681916)
can you explain why you're sleeping _after_ reading `last_write_time`? If I understand correctly, based on the other comments in this thread, the sleep is necessary because the file modified time may not be updated immediately after the contents are written. This test does pass on my linux machine (which has a significantly shorter resolution time), yet I feel that we should be sleeping immediately after the potential file write does/does not occur before checking the file time and verifying it?
...

💬 pinheadmz commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1510029147)
concept ACK, seems like a great idea.

💬 carnhofdaki commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1510084025)
> If anyone is interested in testing, I am running this for a custom signet.
>
> ```
> [signet]
> signetchallenge=512102f7561d208dd9ae99bf497273e16f389bdbd6c4742ddb8e6b216e64fa2928ad8f51ae
> signetblocktime=30
> addnode=45.79.52.207:38333
> dnsseed=0
> ```
>
> Block explorer: https://mutinynet.com faucet: https://faucet.mutinynet.com
>
> There is also a rapid gossip sync server and some lightning nodes as well

Testing synchronization with an unpatched, though rececnt `master`.
...

💬 carnhofdaki commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1510089291)
Oops :) That was mainnet.

The unpatched node gets stuck at block 4000 of Mutinynet:

```
2023-04-16T05:42:04Z UpdateTip: new best=00000139b753b743c4f78b29104c511774447e1d2d9977fde03916d5125b04b3 height=4000 version=0x20000000 log2_work=34.172249 tx=4018 date='2023-04-14T10:01:11Z' progress=1.000000 cache=0.5MiB(4020txo)
2023-04-16T05:42:23Z Adding fixed seeds as 60 seconds have passed and addrman is empty for at least one reachable network
2023-04-16T05:42:23Z Added 0 fixed seeds from re
...

💬 carnhofdaki commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1510139984)
`master` + this little patch (cherry-picked) runs well

```
2023-04-16T07:24:34Z Saw new header hash=000001fda3c4b1118e704e6951a6064d983b2a389bf33cb9c6db2665faf81d20 height=8655
2023-04-16T07:24:34Z [net] Saw new cmpctblock header hash=000001fda3c4b1118e704e6951a6064d983b2a389bf33cb9c6db2665faf81d20 peer=0
2023-04-16T07:24:34Z UpdateTip: new best=000001fda3c4b1118e704e6951a6064d983b2a389bf33cb9c6db2665faf81d20 height=8655 version=0x20000000 log2_work=35.285589 tx=8682 date='2023-04-16T07:24
...

✅ fanquake closed a pull request: "Create docker-image.yml"
(https://github.com/bitcoin/bitcoin/pull/27470)

📝 fanquake locked a pull request: "Create docker-image.yml"
(https://github.com/bitcoin/bitcoin/pull/27470)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

🤔 theStack requested changes to a pull request: "bugfix: rest: avoid segfault for invalid URI"
(https://github.com/bitcoin/bitcoin/pull/27468#pullrequestreview-1386991939)
It's still possible to provoke a segfault via the `/rest/mempool` endpoint, which also uses `GetQueryParameter()` (tested on a `bitcoind -signet -rest` instance):

```
$ curl localhost:38332/rest/mempool/contents.json?%
curl: (52) Empty reply from server

bitcoind output:
libc++abi: terminating with uncaught exception of type std::runtime_error: URI parsing failed, it likely contained RFC 3986 invalid characters
Abort trap (core dumped)
```

Should be easy to fix by catching ` std::ru
...

⚠️ Pttn opened an issue: "Mishandled "unknown" Address Type"
(https://github.com/bitcoin/bitcoin/issues/27472)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Bitcoin Core behaves unexpectedly or even crashes.

* `getnewaddress "" unknown`: "Error: No unknown addresses available. (code -12)"
* `getrawchangeaddress unknown`: "Error: No unknown addresses available. (code -12)"
* `createmultisig 2 '["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd", "03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]' unknow
...

📝 Pttn opened a pull request: "Properly handle "unknown" Address Type"
(https://github.com/bitcoin/bitcoin/pull/27473)
Fixes https://github.com/bitcoin/bitcoin/issues/27472 by also handling at the relevant places the case where ParseOutputType returns OutputType::UNKNOWN, and not just when it returns `std::nullopt`.

💬 achow101 commented on pull request "Properly handle "unknown" Address Type":
(https://github.com/bitcoin/bitcoin/pull/27473#issuecomment-1510495876)
Given that we shouldn't ever accept `unknown` as an output type, I think it would make sense to just make `ParseOutputType` to never return `OutputType::UNKNOWN` instead of having every caller handle it specially.

💬 Pttn commented on pull request "Properly handle "unknown" Address Type":
(https://github.com/bitcoin/bitcoin/pull/27473#issuecomment-1510498491)
Makes sense, commit updated.

💬 pablomartin4btc commented on pull request "bugfix: rest: avoid segfault for invalid URI":
(https://github.com/bitcoin/bitcoin/pull/27468#issuecomment-1510527803)
> It's still possible to provoke a segfault via the `/rest/mempool` endpoint, which also uses `GetQueryParameter()` (tested on a `bitcoind -signet -rest` instance):
>

Thanks @theStack. I'll fix it asap.

💬 ariard commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1510569136)
I think one more sub-category to track is the feedback collected on how package
relay/nversion=3 are fitting multi-party applications and contracting protocols
- Lightning: legacy channels / anchor output / taproot types
- Peerswap: claim with preimage + refund cooperatively + refund after csv passes
- Collaborative transaction: splicing + dual funding flows

The [0-conf flow](https://github.com/lightning/bolts/pull/910) of Lightning can be also considered. As deployed today by some LSPs w
...

💬 ajtowns commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1510719846)
I think at a high level, the "package relay" part looks like:

- [x] make package mempool acceptance available via rpc in regtest
- [ ] fix up package acceptance/retention/mining policies to be free from DoS vectors and to behave sensibly
- [ ] behave sensibly with arbitrary txs in a package
- [ ] implement bip331 to expose package relay over p2p not just rpc

And relatedly:

- [ ] version 3 acceptance/rbf policies to limit pinnability
- [ ] ephemeral anchors

Particularly if #
...

💬 ajtowns commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1510743503)
ACK 6fb01d26c57f6af7205721e528bbafee8fa41027

👍 vasild approved a pull request: "p2p: skip netgroup diversity follow-up"
(https://github.com/bitcoin/bitcoin/pull/27467#pullrequestreview-1387402427)
ACK 51bc09089b18afaa3ba5a1bea638c158df20bdfa

💬 vasild commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#discussion_r1168268562)
nit: `_set` seems unnecessary. E.g. `apples` vs `apples_vector` (I guess the first one is preferable).

👍 vasild approved a pull request: "bugfix: rest: avoid segfault for invalid URI"
(https://github.com/bitcoin/bitcoin/pull/27468#pullrequestreview-1387411349)
ACK 827b14c33f39131ede35ddecde75cc052d977ec5