💬 hodlinator commented on pull request "refactor: Make uint256S(const char*) consteval":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2211455306)
> > **transaction_identifier.h** - Fixed dormant bug in `TxidFromString()` where the `string_view` length wasn't respected(!).
>
> This is known, see [#28922 (comment)](https://github.com/bitcoin/bitcoin/pull/28922#discussion_r1404437378). Thanks for picking it up!
>
> Maybe submit the fix first?

Aha, good that you are tracking it! I see at least 4 possible fixes:

1. Switch back to `Txid TxidFromString(const std::string& str)`
2. Make `TxidFromString()` convert from `std::string_view
...

📝 hodlinator converted_to_draft a pull request: "refactor: Make uint256S(const char*) consteval"
(https://github.com/bitcoin/bitcoin/pull/30377)
`uint256S()` calls taking C-string literals are littered throughout the code and were executed at runtime to perform parsing unless a given optimizer was surprisingly efficient. While this may not be a hot spot, it's better hygiene *in C++20* to store the parsed data blob directly in the binary, without any parsing at runtime.

**transaction_identifier.h** - Fixed dormant bug in `TxidFromString()` where the `string_view` length wasn't respected(!).

Added runtime overload taking `std::string
...

⚠️ achow101 opened an issue: "Double lock detected in `Warnings::GetMessages()`"
(https://github.com/bitcoin/bitcoin/issues/30400)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Running `bitcoin-qt -testnet` results in it eventually deadlocking due to a double lock in the warnings code.

### Expected behaviour

Should not double lock.

### Steps to reproduce

1. Build with `--enable-debug` (or define `DEBUG_LOCKORDER`
2. Run `bitcoin-qt -testnet`, and either sync or reindex
3. It will assert and crash on its own (without `DEBUG_LOCKORDER`, it will eventually han
...

💬 achow101 commented on issue "Double lock detected in `Warnings::GetMessages()`":
(https://github.com/bitcoin/bitcoin/issues/30400#issuecomment-2211502915)
cc @stickies-v as you were the last to touch the warnings code.

📝 tdb3 opened a pull request: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401)
The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251).
The current behavior inconsistently treats blank `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params.

Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting.
Continuation of the upforgrabs PR #29141.

💬 tdb3 commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#issuecomment-2211569527)
## Behavior (master (bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8) vs PR):

### CLI:
1) one valid rpcauth
```
src/bitcoind -regtest -rpcauth=testuser1:798e64e1173b5fa3a5946fbd42ced60e\$8472c4ab034459f7c83f59d94f26c820179455946023fa77a38bc51c785573c4
```
current: bitcoind starts, rpcauth usable
new: no change

2) one empty rpcauth
```
src/bitcoind -regtest -rpcauth
src/bitcoind -regtest -rpcauth=
src/bitcoind -regtest -rpcauth=""
```
current: bitcoind starts
new: bitcoind fails, `Inv
...

💬 tdb3 commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#issuecomment-2211587071)
`test each commit` CI fails on f6d0df30a460db75a944ccea38dfb9fd0827042b to help show current behavior. I can swap the order of the commits to make it pass if need be.

https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251
> it would be a lot more obvious what current behavior is and how the bugfix changes it.

👋 tdb3's pull request is ready for review: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401)

⚠️ achow101 opened an issue: "Illegal Instruction in `CoinStatsIndex::CustomAppend`"
(https://github.com/bitcoin/bitcoin/issues/30402)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

While reindexing a signet node with all indexes enabled, SIGILL occurs and Bitcoin Core crashes.

### Expected behaviour

Not crash

### Steps to reproduce

Do `bitcoind -signet -coinstatsindex -reindex`.

Unsure if this would happen on a sync from scratch or if the index did not previously exist (I'd assume so though).

### Relevant log output

```
Thread 76 "b-scheduler" received sign
...

💬 mzumsande commented on issue "Illegal Instruction in `CoinStatsIndex::CustomAppend`":
(https://github.com/bitcoin/bitcoin/issues/30402#issuecomment-2211650887)
At what height did it crash? If the height was 112516, this is probably a duplicate of #26362.

💬 sipa commented on pull request "random: add benchmarks and drop unnecessary Shuffle function":
(https://github.com/bitcoin/bitcoin/pull/30396#discussion_r1667364813)
Fixed.

💬 sipa commented on pull request "random: add benchmarks and drop unnecessary Shuffle function":
(https://github.com/bitcoin/bitcoin/pull/30396#discussion_r1667364824)
Fixed, here and elsewhere.

✅ fanquake closed a pull request: "Bugfix: RPC: Check for blank rpcauth on a per-param basis"
(https://github.com/bitcoin/bitcoin/pull/29141)

💬 fanquake commented on pull request "Bugfix: RPC: Check for blank rpcauth on a per-param basis":
(https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-2211772901)
See #30401.

💬 tdb3 commented on issue "Illegal Instruction in `CoinStatsIndex::CustomAppend`":
(https://github.com/bitcoin/bitcoin/issues/30402#issuecomment-2211787029)
Performed an IBD on bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8 from scratch (no `signet` dir, so no initial index). Did not see the issue. Performing reindex now.

📝 luke-jr opened a pull request: "GUI/OptionsDialog: Allow Maximize of window"
(https://github.com/bitcoin-core/gui/pull/826)

👍 luke-jr approved a pull request: "Show maximum mempool size in information window"
(https://github.com/bitcoin-core/gui/pull/825#pullrequestreview-2161543405)
tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb & in Knots

💬 alfonsoromanz commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#issuecomment-2211804813)
Post-merge question: Shouldn't this TODO be deleted because of the new test introduced in this PR?

> - TODO: Valid snapshot file, but referencing a snapshot block that turns out to be
> invalid, or has an invalid parent

The `test_snapshot_block_invalidated` function seems to address this scenario by invalidating the snapshot base block and one of its parents, ensuring the snapshot cannot be loaded if the base block or its parent is invalid.

✅ achow101 closed an issue: "Illegal Instruction in `CoinStatsIndex::CustomAppend`"
(https://github.com/bitcoin/bitcoin/issues/30402)

💬 achow101 commented on issue "Illegal Instruction in `CoinStatsIndex::CustomAppend`":
(https://github.com/bitcoin/bitcoin/issues/30402#issuecomment-2211805935)
> At what height did it crash? If the height was 112516, this is probably a duplicate of #26362.

Ah, indeed.