💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2228709837)
> @pablomartin4btc Did you consider [bitcoinknots/bitcoin#83](https://github.com/bitcoinknots/bitcoin/issues/83)?
Yeah, in fact the "switch to Overview" tab when changing/ selecting wallets, was introduced in #718, I can fix it here in a 2nd. commit which will do soon.
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2228709837)
> @pablomartin4btc Did you consider [bitcoinknots/bitcoin#83](https://github.com/bitcoinknots/bitcoin/issues/83)?
Yeah, in fact the "switch to Overview" tab when changing/ selecting wallets, was introduced in #718, I can fix it here in a 2nd. commit which will do soon.
💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2228726277)
CI failure is unrelated.
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2228726277)
CI failure is unrelated.
🤔 pablomartin4btc reviewed a pull request: "fix: rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin-core/gui/pull/828#pullrequestreview-2177986863)
Concept ACK - I'll test it soon.
There's a typo in the commit body ("missleading"), since you are there, perhaps a better prefix for its title could be "`gui:`" and a <ins>nit</ins> on the text could be:
```
The comment in the code regarding the use of an "&"
on a menu item is misleading. If a wallet name has an "&" in it,
it is not supposed to be interpreted as a hot-key, but it should be
shown as it is without replacing it to an underscore.
```
(https://github.com/bitcoin-core/gui/pull/828#pullrequestreview-2177986863)
Concept ACK - I'll test it soon.
There's a typo in the commit body ("missleading"), since you are there, perhaps a better prefix for its title could be "`gui:`" and a <ins>nit</ins> on the text could be:
```
The comment in the code regarding the use of an "&"
on a menu item is misleading. If a wallet name has an "&" in it,
it is not supposed to be interpreted as a hot-key, but it should be
shown as it is without replacing it to an underscore.
```
💬 hebasto commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2228739191)
@pablomartin4btc
> ... was introduced in #718...
Sure about PR number? ('cause there is no such a number in this repo)
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2228739191)
@pablomartin4btc
> ... was introduced in #718...
Sure about PR number? ('cause there is no such a number in this repo)
📝 paplorinc converted_to_draft a pull request: "optimization: Precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher"
(https://github.com/bitcoin/bitcoin/pull/30442)
Continuing the IBD-related micro-optimizations (started in https://github.com/bitcoin/bitcoin/pull/30326), here I'm precalculating the SipHash constants XOR with k0 and k1 for the map hash calculations and short-circuit `COutPoint` equality check for when collisions happen.
before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1.01 | 993,139,589.12 | 0.1%
...
(https://github.com/bitcoin/bitcoin/pull/30442)
Continuing the IBD-related micro-optimizations (started in https://github.com/bitcoin/bitcoin/pull/30326), here I'm precalculating the SipHash constants XOR with k0 and k1 for the map hash calculations and short-circuit `COutPoint` equality check for when collisions happen.
before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1.01 | 993,139,589.12 | 0.1%
...
💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1678010139)
yeah, much simpler. done.
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1678010139)
yeah, much simpler. done.
✅ achow101 closed an issue: "scriptPubKey no address"
(https://github.com/bitcoin/bitcoin/issues/30450)
(https://github.com/bitcoin/bitcoin/issues/30450)
💬 achow101 commented on issue "scriptPubKey no address":
(https://github.com/bitcoin/bitcoin/issues/30450#issuecomment-2228784230)
Inputs do not have scriptPubKeys.
(https://github.com/bitcoin/bitcoin/issues/30450#issuecomment-2228784230)
Inputs do not have scriptPubKeys.
💬 hebasto commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2228793605)
Concept ACK.
> Additionally, this PR moves the `CreatedTransactionResult` struct into its own file.
This part of the PR description seems no longer relevant.
> Thanks for the review ryanofsky!
>
> > I just noticed [bitcoin/bitcoin#25269](https://github.com/bitcoin/bitcoin/pull/25269), which I hadn't seen when I posted my comment above. It looks like that PR was closed for inactivity but it does look like a good approach that solves the issue I didn't like in my comment. (Though maybe
...
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2228793605)
Concept ACK.
> Additionally, this PR moves the `CreatedTransactionResult` struct into its own file.
This part of the PR description seems no longer relevant.
> Thanks for the review ryanofsky!
>
> > I just noticed [bitcoin/bitcoin#25269](https://github.com/bitcoin/bitcoin/pull/25269), which I hadn't seen when I posted my comment above. It looks like that PR was closed for inactivity but it does look like a good approach that solves the issue I didn't like in my comment. (Though maybe
...
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2228808132)
I also incorporated the new `waitFeesChanged()` mining interface method from #3044, which required a bit an overhaul: I ended up adding a second event loop `ThreadSv2MempoolHandler` that deals exclusively with mempool updates.
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2228808132)
I also incorporated the new `waitFeesChanged()` mining interface method from #3044, which required a bit an overhaul: I ended up adding a second event loop `ThreadSv2MempoolHandler` that deals exclusively with mempool updates.
💬 hebasto commented on pull request "Correct tooltip wording for watch-only wallets":
(https://github.com/bitcoin-core/gui/pull/792#issuecomment-2228809247)
Closing for now. Please feel free to reopen it when a non-draft version is ready.
(https://github.com/bitcoin-core/gui/pull/792#issuecomment-2228809247)
Closing for now. Please feel free to reopen it when a non-draft version is ready.
✅ hebasto closed a pull request: "Correct tooltip wording for watch-only wallets"
(https://github.com/bitcoin-core/gui/pull/792)
(https://github.com/bitcoin-core/gui/pull/792)
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2228822746)
@ryanofsky thanks! I'll look into using your patch. After that I'll check where @ismaelsadeeq's feedback still applies.
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2228822746)
@ryanofsky thanks! I'll look into using your patch. After that I'll check where @ismaelsadeeq's feedback still applies.
💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2228835080)
Rebased, and reworked the changes here, to hopefully make them easier to review.
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2228835080)
Rebased, and reworked the changes here, to hopefully make them easier to review.
💬 andrewtoth commented on pull request "optimization: Precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-2228844594)
> Could you please send me the exact command and compiler versions you've used?
`gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) `
I ra
`./configure --enable-bench`
`make`
`./src/bench/bench_bitcoin -filter=.*Out[pP]oint.*`
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-2228844594)
> Could you please send me the exact command and compiler versions you've used?
`gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04) `
I ra
`./configure --enable-bench`
`make`
`./src/bench/bench_bitcoin -filter=.*Out[pP]oint.*`
💬 theuni commented on pull request "depends: remove Darwin ENV unsetting":
(https://github.com/bitcoin/bitcoin/pull/30451#issuecomment-2228853767)
Whoa, this greatly simplifies things. When/why where these removed in guix? I thought they were pretty fundamental there.
I wonder if we should check for the use of these in depends and bail out if they're set though.
(https://github.com/bitcoin/bitcoin/pull/30451#issuecomment-2228853767)
Whoa, this greatly simplifies things. When/why where these removed in guix? I thought they were pretty fundamental there.
I wonder if we should check for the use of these in depends and bail out if they're set though.
👍 tdb3 approved a pull request: "net: Allow -proxy=[::1] on nodes with IPV6 lo only"
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178157576)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
Tested with a similar approach to https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2151672225
Created an LXC container with an external and loopback IPv4 address, and with a loopback IPv6 address.
```
root@test30245:~/bitcoin# ip a | grep inet
inet 127.0.0.1/8 scope host lo
inet6 ::1/128 scope host noprefixroute
inet 192.168.10.81/24 brd 192.168.10.255 scope global eth0
```
Inserted some extra log statments
...
(https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2178157576)
ACK 23333b7ed243071c9b4e4f04c727556d8065acbb
Tested with a similar approach to https://github.com/bitcoin/bitcoin/pull/30245#pullrequestreview-2151672225
Created an LXC container with an external and loopback IPv4 address, and with a loopback IPv6 address.
```
root@test30245:~/bitcoin# ip a | grep inet
inet 127.0.0.1/8 scope host lo
inet6 ::1/128 scope host noprefixroute
inet 192.168.10.81/24 brd 192.168.10.255 scope global eth0
```
Inserted some extra log statments
...
🚀 fanquake merged a pull request: "test: [refactor] Pass TestOpts"
(https://github.com/bitcoin/bitcoin/pull/30407)
(https://github.com/bitcoin/bitcoin/pull/30407)
💬 fanquake commented on pull request "test: Add arguments for creating a slimmer TestingSetup":
(https://github.com/bitcoin/bitcoin/pull/30399#discussion_r1678096936)
#30407 is now merged.
(https://github.com/bitcoin/bitcoin/pull/30399#discussion_r1678096936)
#30407 is now merged.
👍 ryanofsky approved a pull request: "log: LogError with FlatFilePos in UndoReadFromDisk"
(https://github.com/bitcoin/bitcoin/pull/30428#pullrequestreview-2178200481)
Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent
(https://github.com/bitcoin/bitcoin/pull/30428#pullrequestreview-2178200481)
Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent