Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ‘ marcofleon approved a pull request: "fuzz: fix key size in `crypter`"
(https://github.com/bitcoin/bitcoin/pull/30373#pullrequestreview-2161255501)
Tested ACK 4383dc90bac1b5def73352fe222f99807d8ca4dd. I ran this:

`FUZZ=crypter src/test/fuzz/fuzz -set_cover_merge=1 -shuffle=0 -prefer_small=1 -use_value_profile=0 ../qa-assets/fuzz_seed_corpus/crypter ../qa-assets/fuzz_seed_corpus/*`

It successfully merges in 1 attempt with no errors.
πŸ“ Deuces9ers opened a pull request: "Rename doc/release-notes/release-notes-0.9.0.md to doc/release-notes/…"
(https://github.com/bitcoin/bitcoin/pull/30398)
…deuces9ers/release-notes-0.9.0.md

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test
...
πŸ“ TheCharlatan opened a pull request: "test: Add arguments for creating a slimmer TestingSetup"
(https://github.com/bitcoin/bitcoin/pull/30399)
This adds arguments to some of the testing setup constructors for creating an environment without networking and a validation interface. This is useful for improving the performance of the utxo snapshot fuzz test, which constructs a new TestingSetup on each iteration.

Using this slimmed down `TestingSetup` in future might also make the tests a bit faster when run in aggregate.
βœ… hebasto closed a pull request: "Rename doc/release-notes/release-notes-0.9.0.md to doc/release-notes/…"
(https://github.com/bitcoin/bitcoin/pull/30398)
πŸ“ hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30398)
…deuces9ers/release-notes-0.9.0.md

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test
...
πŸ’¬ 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.