💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1202844909)
make sure to check the other call locations, but they aren't supposed to

💬 achow101 commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1560024143)
ACK 272eb5561667482f8226bcf98eea00689dccefb8

✅ achow101 closed an issue: "failure in feature_bip68_sequence.py"
(https://github.com/bitcoin/bitcoin/issues/27129)

🚀 achow101 merged a pull request: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177)

💬 jamesob commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1560035238)
New bug, albeit pretty minor:

- [ ] if you restart the node after activating the snapshot but before connecting any blocks to the snapshot chainstate, `ReadBlockFromDisk` checks fail when calling `VerifyDB()` during chainstate initialization. I'm not sure when this would happen in practice, but it's an annoying edge case.

💬 schjonhaug commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1560053023)
Looks like `>>` is missing after `"maxconnections=2"` in

```
$ cd $DATA_DIR && echo "signet=1" >> bitcoin.conf && echo "maxconnections=2" bitcoin.conf && cd ~
```

Also, it’s easier to copy the commands using the (hover) copy icon <img width="46" alt="image" src="https://github.com/bitcoin/bitcoin/assets/84225/ea868344-91f5-4e4b-b82c-5628f05a88bc"> on Github if you remove the `$` in front.

💬 theuni commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1202937682)
Would it be possible to keep pointers/references to internal data out of this interface from the beginning? I'm not sure if that's a goal at this layer.

For blocktip for example, taking a quick look around, for the ui it looks like this eventually gets converted to:
`void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)`

I wonder if we can do the same here?

🤔 theuni reviewed a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1440486632)
From IRC:

> <TheCharlatan> Hey, how about moving the last two commits of #27636 (the only ones to touch the shutdown logic) to #27711? Then we can get #27636 merged and move ahead with #27576.

This is a good idea imo. I really don't want to slow this down, but I would benefit from a few extra days of catching up and testing the shutdown logic before reviewing/critiquing it in detail.

📝 fjahr opened a pull request: "init: Improve file descriptor limit handling"
(https://github.com/bitcoin/bitcoin/pull/27730)
I was playing with file descriptor limits to test another issue saw the following warning printed when I had configured a file descriptor limit of 150:

```
Warning: Reducing -maxconnections from 125 to -9, because of system limitations.
```

The warning could have been resolved with a one-line change but I found the surrounding code hard to reason about at first, so I added a few comments and reorganized the code a bit for better clarity.

Aside from the warning there is one further beh
...

💬 schjonhaug commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1560102597)
A comment on the following paragraphs:

> To create a non-witness transaction of 64 bytes or so, You can generate an OP_RETURN output with the following dummy data in hexadecimal format: 6a00000000.

> Get a new change address
> $ bcli -regtest getnewaddress
> You should create a transaction with two outputs: one that sends Bitcoin to the address you generated, and the other that is an OP_RETURN output with the hex value of 6a00000000

> $ bcli -regtest createrawtransaction '[{"txid":"04
...

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1560143021)
Updated per feedback, thanks @ishaanam.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1203034031)
pushed

💬 achow101 commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#discussion_r1203035210)
In 8218ce6b02b24eeef31f4e343ea777b7844907fc "wallet: return and display signer error"

Returning the error in an optional feels a bit wrong since it inverts are typical pattern. Rather than that the result is equivalent to `true`, we have to check that it's equivalent to `false`.

I think it would be better to use `util::Result<bool>` instead of an optional in order to preserve our typical error checking pattern.

💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1560158824)
ACK 76396ca376188631ba46bd47b134881efcc6f755

👍 brunoerg approved a pull request: "rpc: Fix invalid bech32 address handling"
(https://github.com/bitcoin/bitcoin/pull/27727#pullrequestreview-1440618244)
crACK eeee55f9288740747b6e8d806ce8177fd92635cf

💬 brunoerg commented on pull request "rpc: Fix invalid bech32 address handling":
(https://github.com/bitcoin/bitcoin/pull/27727#discussion_r1203063730)
feel free to ignore:

since it's just 1 node
```suggestion
self.extra_args = [["-prune=899"]]
```

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1203079320)
Primarily, I did not want to mess with existing interfaces, but I checked through everything and it would be feasibly to entirely drop usage of `CBlockIndex` for this notification. The change is a bit sprawling though and also ends up touching rpc code.

> I'm not sure if that's a goal at this layer.

The way I see this play out during the next phase of exploring a more suitable kernel api, these notifications might be used to feed back into the `ChainstateManager`. As long as the chainman
...

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1560199240)
Dropped bbfbcd0360e486d47025a412f2c0f4e8535ab255 -> 7d3b35004b039f2bd606bb46a540de7babdbc41e ([chainstateRmKernelUiInterface_12](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_12) -> [chainstateRmKernelUiInterface_13](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_12..chainstateRmKernelUiInterface_13).

This reverts the last two commits touc
...

🤔 mzumsande reviewed a pull request: "index: prevent race by calling 'CustomInit' prior setting 'synced' flag"
(https://github.com/bitcoin/bitcoin/pull/27720#pullrequestreview-1440641924)
Code review ACK 3126454dcfa1dd29bb66500d5f2b5261684d6c58

📝 fjahr opened a pull request: "Prevent file descriptor exhaustion from too many RPC calls"
(https://github.com/bitcoin/bitcoin/pull/27731)
Fixes #11368

This requires libevent 2.2.1 which so far was only [released in alpha](https://github.com/libevent/libevent/releases/tag/release-2.2.1-alpha).

For more context see specifically https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-632717903, this uses the feature mentioned there as libevent#592.