💬 TheCharlatan commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538539627)
I'll drop the commit. Could this be related to https://github.com/bitcoin/bitcoin/issues/27586 and solve some of the performance issues?
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1538539627)
I'll drop the commit. Could this be related to https://github.com/bitcoin/bitcoin/issues/27586 and solve some of the performance issues?
💬 Sjors commented on pull request "rpc: distinguish between vsize and sigop-adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187505606)
I would call this `adjusted_vsize`. Or `vvsize` :-P
(https://github.com/bitcoin/bitcoin/pull/27591#discussion_r1187505606)
I would call this `adjusted_vsize`. Or `vvsize` :-P
📝 hebasto opened a pull request: "bench: Add `-sha-implementation` command-line option"
(https://github.com/bitcoin/bitcoin/pull/27598)
On the master branch, only the best available `SHA256` implementation is being benchmarked. This PR allows to benchmark different ones.
For example,
```
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=standard
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=sse4
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=sse4,avx2
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=all
```
Found it useful, while working on https://github.c
...
(https://github.com/bitcoin/bitcoin/pull/27598)
On the master branch, only the best available `SHA256` implementation is being benchmarked. This PR allows to benchmark different ones.
For example,
```
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=standard
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=sse4
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=sse4,avx2
./src/bench/bench_bitcoin -filter=SHA256.* -sha-implementation=all
```
Found it useful, while working on https://github.c
...
💬 MarcoFalke commented on pull request "bench: Add `-sha-implementation` command-line option":
(https://github.com/bitcoin/bitcoin/pull/27598#issuecomment-1538554964)
What about benchmarking all that are available on the system?
(https://github.com/bitcoin/bitcoin/pull/27598#issuecomment-1538554964)
What about benchmarking all that are available on the system?
💬 Sjors commented on issue "rpc: show P2(W)SH redeemScript in getrawtransaction (and friends)":
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1538555679)
I think adding fields is fine, unless determining their values is slow for some reason.
(https://github.com/bitcoin/bitcoin/issues/27391#issuecomment-1538555679)
I think adding fields is fine, unless determining their values is slow for some reason.
💬 hebasto commented on pull request "bench: Add `-sha-implementation` command-line option":
(https://github.com/bitcoin/bitcoin/pull/27598#issuecomment-1538558986)
> What about benchmarking all that are available on the system?
You mean, as separated benchmarks? Without `-sha-implementation` option?
(https://github.com/bitcoin/bitcoin/pull/27598#issuecomment-1538558986)
> What about benchmarking all that are available on the system?
You mean, as separated benchmarks? Without `-sha-implementation` option?
👍 furszy approved a pull request: "Implement Mini version of BlockAssembler to calculate mining scores"
(https://github.com/bitcoin/bitcoin/pull/27021#pullrequestreview-1417018318)
ACK 6b605b91 modulo `miniminer_overlap` test.
Not really blocking, I'm planning to go deeper later. And probably add some explanatory comments and code simplifications.
(https://github.com/bitcoin/bitcoin/pull/27021#pullrequestreview-1417018318)
ACK 6b605b91 modulo `miniminer_overlap` test.
Not really blocking, I'm planning to go deeper later. And probably add some explanatory comments and code simplifications.
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582119)
Done
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582119)
Done
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582254)
Done
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582254)
Done
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582474)
Done
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187582474)
Done
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187585622)
I just changed the error message and adopted the diff (seems so much better to check whether this is a `nullptr`):
```diff
- if (!output_connection_direction) {
+ if (output_connection_direction == nullptr) {
+ // Only NetWhitebindPermissions() should pass a nullptr.
```
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187585622)
I just changed the error message and adopted the diff (seems so much better to check whether this is a `nullptr`):
```diff
- if (!output_connection_direction) {
+ if (output_connection_direction == nullptr) {
+ // Only NetWhitebindPermissions() should pass a nullptr.
```
💬 hebasto commented on pull request "refactor: Remove unused GetTimeMillis":
(https://github.com/bitcoin/bitcoin/pull/27594#issuecomment-1538588861)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/27594#issuecomment-1538588861)
Concept ACK.
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187591763)
Added right before:
```diff
+ # Whitelist peers to speed up tx relay / mempool sync
```
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187591763)
Added right before:
```diff
+ # Whitelist peers to speed up tx relay / mempool sync
```
💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187594094)
If `CreateCoins` generates a coin that is already inside any of the results, `AddInputs` will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.
Same apply for the fuzz `Merge` function commit.
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187594094)
If `CreateCoins` generates a coin that is already inside any of the results, `AddInputs` will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.
Same apply for the fuzz `Merge` function commit.
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187595794)
Just added it to make it clearer
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1187595794)
Just added it to make it clearer
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1538611034)
Force-pushed addressing @jonatack's review.
- Moved `InitializePermissionFlags` out of `CConnman`
- Made `TryParsePermissionFlags` static
- Added more test coverage https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186367052
- Improved documentation
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1538611034)
Force-pushed addressing @jonatack's review.
- Moved `InitializePermissionFlags` out of `CConnman`
- Made `TryParsePermissionFlags` static
- Added more test coverage https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186367052
- Improved documentation
💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187606760)
> If CreateCoins generates a coin that is already inside any of the results, AddInputs will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.
Yes, that's why it calls `CreateCoins` twice and with different "utxos pool". Is there any risk yet?
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187606760)
> If CreateCoins generates a coin that is already inside any of the results, AddInputs will throw a runtime error, which the fuzzing framework will detect as a failure when it is not. It is part of the class boundaries.
Yes, that's why it calls `CreateCoins` twice and with different "utxos pool". Is there any risk yet?
💬 Sjors commented on pull request "Update best_header inside Connect/DisconnectTip":
(https://github.com/bitcoin/bitcoin/pull/26260#issuecomment-1538638453)
I deployed this patch on the two ForkMonitor (mirror) nodes that frequently use `invalidateblock` and `reconsiderblock`. It's cherry-picked on top of the v25.0rc1 and v24.1rc2 tags. Typically we get about one error message per week where the block it rewinds to is not the one we expected. We'll see if that stops.
(https://github.com/bitcoin/bitcoin/pull/26260#issuecomment-1538638453)
I deployed this patch on the two ForkMonitor (mirror) nodes that frequently use `invalidateblock` and `reconsiderblock`. It's cherry-picked on top of the v25.0rc1 and v24.1rc2 tags. Typically we get about one error message per week where the block it rewinds to is not the one we expected. We'll see if that stops.
💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187625261)
No ok. The reason why this doesn't fails is the test global increasing `next_locktime` which ensures that all UTXOs receive a different hash even when they have the same data.
So, nvm. A comment wouldn't hurt but nothing serious.
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1187625261)
No ok. The reason why this doesn't fails is the test global increasing `next_locktime` which ensures that all UTXOs receive a different hash even when they have the same data.
So, nvm. A comment wouldn't hurt but nothing serious.
💬 achow101 commented on pull request "Switch hardened derivation marker to h":
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1538744152)
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
(https://github.com/bitcoin/bitcoin/pull/26076#issuecomment-1538744152)
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c