💬 MarnixCroes commented on pull request "gui: fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#discussion_r1583469857)
- it doesn't work for P2SH either, which doesn't start with bc1/tb1

- I'd probably leave out the testnet/signet example/mentioning of tb1, to prevent confusion, or remove `addresses staring with...` in full

- nit: maybe write segwit with capitals S and W as used in the rest of the UI

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2083440214)
cc @dongcarl

💬 AngusP commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2083451948)
tACK e233ef18364a5758adcb83eea53bdf92aa1bf551

💬 laanwj commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2083468516)
Changing the system date is a terrible workaround in any case imo. Is there really no other way?

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583635647)
done.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583636377)
Yes, I think so. We later may increase nFile and then compare it to `last_blockfile` (line `if (nFile != last_blockfile)`)

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583637879)
I took both from FindBlockPos, snake case is correct, but for historical reasons camel case is still used in lots of places.
But since this is arguably new code I renamed `nAddSize` to `added_size`

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583638770)
You mean the comment "The ASSUMED state is initialized, when necessary, in FindBlockPos()."?
That behavior is unchanged, it's part of the normal usage of `FindBlockPos` and wasn't happening during reindex anyway (see first commit), so I don't think it needs to be updated.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583638866)
done

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583639069)
done

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583640596)
Yes. I added this in the first commit, so that the second commit doesn't change behavior.

As for your second q: Well, it'd be undefined behavior if `m_blockfile_cursors` didn't haven an element for `BlockfileType::NORMAL`. On the other hand, `m_blockfile_cursors[...]` is used all over the place, I'm not sure if we want to have an assert for each occurence. Other opinions?

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1583642139)
I updated some comments. The entire test setup probably makes a bit less sense after the refactor, users unfamiliar
with the history might ask themselves why Reindex / AddToBlockFileInfo would change the block files
so that we'd require a test making sure it doesn't.

💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#issuecomment-2083526332)
[39ad8d8 ](https://github.com/bitcoin/bitcoin/commit/39ad8d825e35b7326ad0ea25c37d3fe12ded64c9)to [194e84a](https://github.com/bitcoin/bitcoin/commit/194e84accced947ef63c6db389bc62a2b58cffa3): Addressed review feedback by @paplorinc, thanks!

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2083538585)
@laanwj I believe newer versions of OpenSSL have more robust tests, but we don't want to bump our Guix Time Machine commit just for that.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2083538787)
> Up for grabs?

Updating in a day or too. Just got over with my final year college exams. Just address your changes and rebase to the main.

Same for the Spend file.

💬 mzumsande commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2083567402)
> Is it possible for other peers to make me believe I have a slow internet connection?

Sorry, I missed this question. For single peers yes, but the idea is that the data would be taken from multiple outbound peers - it is hard for an attacker to control multiple of these, see [eclipse attacks](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Addrman-and-eclipse-attacks).

Will soon come back to this PR and take it out of draft.

📝 glozow opened a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000)
Built on top of #28970, makes it more robust.

Currently the main map in `TxOrphanage` is indexed by txid; we do not allow 2 transactions with the same txid into TxOrphanage. This means that if we receive a transaction and want to store it in orphanage, we'll fail to do so if a same-txid-different-witness version of the tx already exists in the orphanage. The existing orphanage entry can stay until it expires 20 minutes later, or until we find that it is invalid.

This means an attacker can
...

💬 hebasto commented on pull request "deploy: remove some tools when cross-compiling for macOS":
(https://github.com/bitcoin/bitcoin/pull/29890#issuecomment-2083695651)
Apparently, CMake expects to find the `install_name_tool` when cross-compiling for macOS.

From https://github.com/hebasto/bitcoin/actions/runs/8885213906/job/24396062348:
```
-- The C compiler identification is Clang 17.0.6
CMake Error at /usr/share/cmake-3.25/Modules/CMakeFindBinUtils.cmake:233 (message):
Could not find install_name_tool, please check your installation.
```

💬 Randy808 commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2083859284)
Here's the commit with the reference changes applied:
https://github.com/Randy808/bitcoin/commit/f39144d808975c016c1a94022a2c4251950afb7d

Repro instructions after branch is built:
```
# Pick a directory to put the previous releases in
export PREVIOUS_RELEASES_DIR=<prev_releases_dir>
test/get_previous_releases.py -b -t "$PREVIOUS_RELEASES_DIR"
test/functional/wallet_backwards_compatibility.py
```

⚠️ WaffleApe opened an issue: "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs"
(https://github.com/bitcoin/bitcoin/issues/30001)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Bitcoin Core launches and starts syncing with the blockchain. After a variable amount of time (20 minutes to 2 hours), Bitcoin Core crashes (app closed) with no error in the logs

### Expected behaviour

Bitcoin Core is expected to sync without crashing

### Steps to reproduce

- launching Bitcoin Core using Bitcoin-qt for graphic interface
- Database cache is set to 4000mb
- After 30 m
...