π 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.
(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
...
(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.
(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)
(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
...
(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
...
(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
...
(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
...
(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.
(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.
(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
...
(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.
(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)
(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
...
(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.
(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.
(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.
(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)
(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.
(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.
(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.