💬 achow101 commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242864916)
Done

💬 achow101 commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242864971)
Done

💬 achow101 commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242865250)
Removed `corrupted_tx` as suggested.

💬 FelixWeis commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1608385210)
```
2023-06-26T19:44:34.554349Z [net] received: tx (313 bytes) peer=44
2023-06-26T19:44:34.554644Z [mempool] AcceptToMemoryPool: peer=44: accepted 783c947208643cb757b916a9693fcbbac89ae4059ec748ff8c78735739c831bc (poolsz 21998 txn, 70199 kB)
2023-06-26T19:44:34.554678Z [net] received: tx (313 bytes) peer=44
2023-06-26T19:44:34.554966Z [mempool] AcceptToMemoryPool: peer=44: accepted ccec8d2d77fcfca2386280efaff17fa5b3011d8bae3b91a2fdd2345a1ee9e5b8 (poolsz 21999 txn, 70200 kB)
2023-06-26T19:44:
...

👍 ryanofsky approved a pull request: "index: improve initialization and pruning violation check"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1499453218)
Code review ACK 3514f768b181d359d0218845ace0266d37e9dfbd

This looks good, and the PR seems pretty easy to review now. It's a series of simple, clean improvements that make index startup more straightforward and efficient.

I left two review comments, one is about a potential bug in an intermediate commit, but it's fixed by the final commit, and also not hard to address if you want to take the suggestion there.

I'd consider maybe renaming the PR from "index: improve initialization and pru
...

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1242783297)
In commit "init: don't start indexes sync thread prematurely" (d7a13cb990826d0422e2135a6fd4f2b8813b6eb8)

I don't think replacing the `&& g_indexes_ready_to_sync` condition with `&& !LoadingBlocks` here in this commit is correct or preserves behavior. The goal of having the condition is to skip checking code and avoid the prune_violation error below if `-reindex-chainstate` is used, but `!LoadingBlocks` will still be true if `-reindex-chainstate` is used. Definition of `LoadingBlocks` is `m_im
...

💬 ryanofsky commented on pull request "index: improve initialization and pruning violation check":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1242830869)
In commit "refactor: simplify pruning violation check" (1f88e942d34bf8c51374567e9086f5535f0bde21)

Looking at the `CheckBlockDataAvailability` and `GetFirstStoredBlock` functions more closely, I don't think is ever a legitmate reason to call them with an upper block that missing data, or to call them with a lower block that is not an ancestor of ancestor of upper block.

I think if either of these things happen, something has gone badly wrong, and it would be better to treat these things as
...

💬 ryanofsky commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#discussion_r1242877722)
Nice, I like the use of first() and last() to avoid the pointer math

👍 ryanofsky approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1499593012)
Code review ACK 7cb774048c5f04b3e20d95e794c5a350ea4eff97. Just more suggested error handling tweaks since last review

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

💬 furszy commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242886714)
```suggestion
// Use std::max as fill_wtx may have already set result to CORRUPT;
```

💬 ryanofsky commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1608423449)
Updated 3fca8c3c7501b7a8e0a3adbbf6c78bb446e60924 -> 2109a147406247bb654ee5fb8421a8594919fbd5 ([`pr/noptr.1`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.1) -> [`pr/noptr.2`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noptr.1..pr/noptr.2)) using `AsBytes` and a `SpanFromDbt` function to avoid `reinterpret_cast` in a few places

💬 dscotese commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1608428937)
I don't yet understand permissions well enough on MacOS, but if you can see /Users/nsa/bitcoin/depends/setup.py and Python can't, checking for the account under which Python runs might be a good idea. Can you invoke /usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/Resources/Python.app/Contents/MacOS/Python from the same place you invoke make?

💬 MarcoFalke commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1608779293)
Can you try with my suggested diff?

```diff
diff --git a/src/net.cpp b/src/net.cpp
index a96ffcfbe9..6c9f928f0d 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -854,6 +854,10 @@ size_t CConnman::SocketSendData(CNode& node) const
node.nSendBytes += nBytes;
node.nSendOffset += nBytes;
nSentSize += nBytes;
+ if (node.nSendOffset > data.size()) {
+ LogPrint(BCLog::NET, "socket sent %s bytes (total: %s, offset: %s) for peer=%d.\n
...

💬 MarcoFalke commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1608855369)
Needs rebase

lgtm ACK 2109a147406247bb654ee5fb8421a8594919fbd5 🐭

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK
...

⚠️ n1kcha opened an issue: "Wallet not loaded"
(https://github.com/bitcoin/bitcoin/issues/27982)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Suddenly wallet not loaded. I was resyncing blockchain (now synced). The wallet is old and has balance. Yesterday there was no problem.
Recently updated for V24 to V25. All was working ok until today. Node is not pruned.

Disk that blocks are stored is external Toshiba with USB connection, i have just a soft link between .bitcoin/blocks to blocks at Toshiba. I have repaired this disk a
...

🤔 S3RK reviewed a pull request: "Bump unconfirmed ancestor transactions to target feerate"
(https://github.com/bitcoin/bitcoin/pull/26152#pullrequestreview-1466867248)
Did a first pass of code review. Overall code looks really good, need to spend more time reviewing the tests though.

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1241791109)
in "Amend bumpfee for inputs with overlapping ancestry" 56139fd808250fcb693b5f185e7e510804a33470

nit: you can drop this method if you decide to have `GetTotalBumpFee()` instead

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1221051466)
in "Amend bumpfee for inputs with overlapping ancestry" 56139fd808250fcb693b5f185e7e510804a33470

nit: while you're here, should we drop earlier calls to `ComputeAndSetWaste` and just call it once for all eligible results before selecting the best one?

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1238118450)
in "Bump unconfirmed parent txs to target feerate" 8e3d924f02ee28043e8844a301389915823e5893

nit: suggestion
```cpp
for (auto& output : result.coins.All()) {
output.ApplyBumpFee(map_of_bump_fees.at(output.outpoint));
}
```