💬 ryanofsky commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1609910681)
Rebased 2109a147406247bb654ee5fb8421a8594919fbd5 -> 650ca0d937c2c90adeeac60adae090902618d771 ([`pr/noptr.2`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.2) -> [`pr/noptr.3`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noptr.2-rebase..pr/noptr.3)) due to conflict with #27479

💬 soheilwikey12 commented on pull request "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate":
(https://github.com/bitcoin/bitcoin/pull/27969#issuecomment-1609942086)
14tDQpPnHqoOFvjBQzKoNM6bgwHL1oklVNLPRQr4Kvauq8oHICNDFvHv

💬 theStack commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1244157426)
Thanks for clarifying, that makes sense.

💬 hebasto commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#discussion_r1244194688)
Unfortunately, using the `-r` option still introduces non-reproducibilty. Diffoscope points at some metadata related to directories. Not sure, but it looks like they are about creation timestamps.

Suggesting to use the same approach as we are using for Windows builds:
```suggestion
find $(APP_DIST_DIR) | sort | zip -X@ $@
```

Two subsequent runs on my Ubuntu 22.04 returned the same hashes.

💬 hebasto commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1610057916)
> As evidence, Visual Studio Code has recently started distributing their macOS builds as ZIP files instead of DMGs, as you can see on their download page: [code.visualstudio.com/Download](https://code.visualstudio.com/Download).

In comparison to UX of downloading `VSCode-darwin.zip`, in the current PR branch, there is a top directory named `dist`, which seems redundant:


💬 0xB10C commented on pull request "test: various USDT functional test cleanups (27831 follow-ups)":
(https://github.com/bitcoin/bitcoin/pull/27944#issuecomment-1610093695)
Thanks for picking this up! Concept ACK. I'll have a closer look.

💬 achow101 commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1610102318)
ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110

🚀 achow101 merged a pull request: "util: implement `noexcept` move assignment & move ctor for `prevector`"
(https://github.com/bitcoin/bitcoin/pull/27334)

🤔 jonatack reviewed a pull request: "refactor: Drop unsafe AsBytePtr function"
(https://github.com/bitcoin/bitcoin/pull/27978#pullrequestreview-1501709807)
ACK 650ca0d937c2c90adeeac60adae090902618d771

💬 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`?