💬 jonatack commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#discussion_r1244251001)
Would it make sense for `SpanFromDbt()` to be a static method in `src/wallet/bdb.cpp` instead, which contains its only callers?

💬 fjahr commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244328552)
This is what I had in mind: https://github.com/fjahr/bitcoin/commit/e26c89251b9fc2ae24c2c7ff90b8fd5490335ee5

💬 furszy commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244338868)
Sure. Added the changes and documented the preconditions.

💬 jonatack commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1610203248)
The merge script picked my quoted github snippet rather than the ACK; maybe it should ignore lines that begin with `> `.

```
ACKs for top commit:
achow101:
ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110
jonatack:
> Tested Approach ACK [bfb9291](https://github.com/bitcoin/bitcoin/commit/bfb9291a8661fe5b26c14ed755cfa89d27c37110), ~ACK modulo re-verifying the implementation of the last commit.
john-moffett:
Approach ACK bfb9291a8661fe5b26c14ed75
...

🤔 furszy reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1501863590)
Updated per feedback, thanks ryanofsky!.

* Reordered code so `g_indexes_ready_to_sync` removal occurs at the last commit.
* Documented the `CheckBlockDataAvailability` preconditions.
* And fixed a little bug that could have arose when the user erases the blocks directory without erasing the indexes directory. In this scenario, the process need to verify that `CheckBlockDataAvailability` return value is the genesis block.

📝 sipa opened a pull request: "Add support for RFC8439 variant of ChaCha20"
(https://github.com/bitcoin/bitcoin/pull/27985)
Based on and replaces part of #25361, part of the BIP324 project (#27634).

There are two variants of ChaCha20 in use. The original one uses a 64-bit nonce and a 64-bit block counter, while the one used in RFC8439 uses a 96-bit nonce and 32-bit block counter. This adds support for the 96/32 variant to our existing ChaCha20 classes.

👍 furszy approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1501871959)
reACK 3c83b1d8

💬 mzumsande commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1243985565)
commit 68b13a2b6c96fd061a31add58b8626c5bf1cdf3d
nit: this is not a struct

💬 mzumsande commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244359758)
commit 68b13a2b6c96fd061a31add58b8626c5bf1cdf3d:
I think that once it's decoupled from `BaseIndex::Init()`, the names `BaseIndex::Start()` and `StartIndexes()` are misleading: It doesn't actually start the indexes, if `Init()` determines that an index is synced, the index is started right then (it begins to process validationinterface signals), and the later `StartIndexes()` does nothing, and could just as well be skipped. I'd prefer something like `IndexBackgroundSync()`.

🤔 mzumsande reviewed a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1501321408)
Concept ACK, didn't review the latest push yet. Only have some issues with the naming.

💬 mzumsande commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244340856)
commit 11fb8fa81ab8e2436e4ae6de79b52581db399265:
This spot doesn't mean the `ThreadImport` function, but the thread. Various other spots below also mean the thread, so I think it'd be better to change this manually, and to also expand the description one line below? I think essentially this thread performs various loading tasks that are part of init but shouldn't block the node from being started, so are done in a separate thread (external block import, reindex, reindex-chainstate, index backg
...

💬 mzumsande commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1244362593)
commit 68b13a2b6c96fd061a31add58b8626c5bf1cdf3d:
Considering how large this function becomes and how low-level the code is in the final state, I don't really love having it in `init.cpp` instead of somewhere in `index/`. Could it be better in `index/base.cpp` as a free function, even if it's not part of `BaseIndex`?

💬 TheCharlatan commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1610260838)
Updated e0653c2223f8d6d74645c9b5742de1c069d8c3a7 -> 0c2fd16275ae078e33d71b0c9da8034075998553 ([kernelInterrupt_6](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_6) -> [kernelInterrupt_7](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInterrupt_6..kernelInterrupt_7)).
* Adjusted and simplified documentation as suggested by @ryanofsky

Rebased 0c2fd16275ae078e33d71b0c9da8034075998553 -> 001acf
...

💬 achow101 commented on pull request "Remove the syscall sandbox":
(https://github.com/bitcoin/bitcoin/pull/27896#issuecomment-1610264637)
ACK 32e2ffc39374f61bb2435da507f285459985df9e

The syscall sandbox has a rather significant maintenance burden for rather limited benefit.

✅ achow101 closed an issue: "Outcome of the syscall sandbox experiment"
(https://github.com/bitcoin/bitcoin/issues/24771)

🚀 achow101 merged a pull request: "Remove the syscall sandbox"
(https://github.com/bitcoin/bitcoin/pull/27896)

👍 ryanofsky approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1501951643)
Code review ACK 3c83b1d884b419adece95b335b6e956e7459a7ef. Just Marco's suggested error handling fixes since last review

🤔 jonatack reviewed a pull request: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1501951235)
Reviewed/built/tested the first four commits up to https://github.com/bitcoin/bitcoin/commit/d2f46c705540c74c2b6f83a66535c3ead1cb95d4.

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244454225)
d2f46c7 Verified the behavior improvement in this commit by changing this line to make it fail.

master

```bash
$ ./src/test/test_bitcoin -t banman_tests
Running 1 test case...
libc++abi: terminating due to uncaught exception of type std::runtime_error: 'Dropping entry with unknown version (2) rom ban list' not found in debug log

******** errors disabling the alternate stack:
#error:22
Invalid argument
unknown location:0: fatal error: in "banman_tests/file": signal: SIGABR
...

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244442031)
Rather than including everything, it might be good to have a separate compilation unit for code that needs to be in `util/setup_common`, versus the current `util/net` code that is only needed for a handful of the tests.

🚀 ryanofsky merged a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914)