Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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
...
🤔 tdb3 reviewed a pull request: "test: Fix intermittent issue in wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/29982#pullrequestreview-2030034185)
Concept ACK. I would still like to review the logic a bit more. Leaving some test activities as notes:

Tested by performing the following:

Fetched previous releases with `test/get_previous_releases.py -b`, ran all (non-extended) functional tests (with `--previous-releases` to prevent skipping). All passed.

As a very quick/basic santity check for intermittency, executed
```$ test/functional/test_runner.py --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp --filter=wallet_backwards_compatib
...
🤔 tdb3 reviewed a pull request: "doc: removed help text saying that peers may not connect automatically"
(https://github.com/bitcoin/bitcoin/pull/29994#pullrequestreview-2030045832)
ACK for 95897ff181c0757e445f0e066a2a590a0a0120d2.

Thank you for circling back to this. Makes sense to me given 23.0+ adoption. Also like how default port values were kept in the message for clarity.

```
$ src/bitcoind -help | grep -A3 " -port="
-port=<port>
Listen for connections on <port> (default: 8333, testnet: 18333, signet:
38333, regtest: 18444). Not relevant for I2P (see doc/i2p.md).
```
💬 maflcko commented on issue "Bitcoin Core 27 crash at sync - Ubuntu - No error in logs":
(https://github.com/bitcoin/bitcoin/issues/30001#issuecomment-2084400902)
> 2Tb HDD for blockchain storage

What filesystem is the storage? Is it attached via USB? Is the datadir on the storage, or only the blocksdir? What is the full bitcoin config file?

Can you run it in gdb again for a traceback?

Does `dmesg` say anything?