β
fanquake closed a pull request: "Improve Chinese translations for peer connection types"
(https://github.com/bitcoin-core/gui/pull/917)
(https://github.com/bitcoin-core/gui/pull/917)
π fanquake opened a pull request: "netbase: Remove "tor" as a network specification"
(https://github.com/bitcoin/bitcoin/pull/34031)
"tor" as a network specification was deprecated in 60dc8e4208 in favor of "onion"
and this commit removes it and updates the relevant test.
Previously #16029. This has been warning as being deprecated since `v0.17.0`.
(https://github.com/bitcoin/bitcoin/pull/34031)
"tor" as a network specification was deprecated in 60dc8e4208 in favor of "onion"
and this commit removes it and updates the relevant test.
Previously #16029. This has been warning as being deprecated since `v0.17.0`.
π¬ fanquake commented on pull request "netbase: Remove "tor" as a network specification":
(https://github.com/bitcoin/bitcoin/pull/16029#issuecomment-3627007328)
Picked up in #34031.
(https://github.com/bitcoin/bitcoin/pull/16029#issuecomment-3627007328)
Picked up in #34031.
π¬ maflcko commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2598713637)
Heh, I wish all of this was written in Rust, so that types are checked at compile-time :sweat_smile:
(https://github.com/bitcoin/bitcoin/pull/33702#discussion_r2598713637)
Heh, I wish all of this was written in Rust, so that types are checked at compile-time :sweat_smile:
π¬ adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2598734691)
these were great suggestions, thank you - added
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2598734691)
these were great suggestions, thank you - added
π¬ hebasto commented on pull request "doc: Add troubleshooting note about Guix on SELinux systems":
(https://github.com/bitcoin/bitcoin/pull/32442#discussion_r2598737786)
I believe this is no longer necessary.
On my Fedora 43 installation with working Guix:
```
$ sestatus
SELinux status: enabled
SELinuxfs mount: /sys/fs/selinux
SELinux root directory: /etc/selinux
Loaded policy name: targeted
Current mode: enforcing
Mode from config file: enforcing
Policy MLS status: enabled
Policy deny_unknown status: allowed
Memory protection checking: actual (secure)
Max kernel
...
(https://github.com/bitcoin/bitcoin/pull/32442#discussion_r2598737786)
I believe this is no longer necessary.
On my Fedora 43 installation with working Guix:
```
$ sestatus
SELinux status: enabled
SELinuxfs mount: /sys/fs/selinux
SELinux root directory: /etc/selinux
Loaded policy name: targeted
Current mode: enforcing
Mode from config file: enforcing
Policy MLS status: enabled
Policy deny_unknown status: allowed
Memory protection checking: actual (secure)
Max kernel
...
π¬ plebhash commented on issue "consider adding a new `interface RawTxFeed` on Mining IPC":
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627141436)
> They could be collected in batches though.
yeah `waitNext` probably shouldn't return one single tx at a time, batching makes more sense
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627141436)
> They could be collected in batches though.
yeah `waitNext` probably shouldn't return one single tx at a time, batching makes more sense
π¬ plebhash commented on issue "consider adding a new `interface RawTxFeed` on Mining IPC":
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627204038)
> fetching specific missing txs just in time via `getTransactionsByWitnessId()`
I feel this should be left as a last-resort kind of thing
whatever feed JDS gets from Bitcoin Core (be it updates with single or batched txs), it should contain full `txdata` instead of only `wtxids`
then it's up to JDS implementation to aim to optimize which txs it could selectively drop to avoid unbounded memory consumption
but `txdata` should be made available sooner, rather than selectively fetched (with extr
...
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627204038)
> fetching specific missing txs just in time via `getTransactionsByWitnessId()`
I feel this should be left as a last-resort kind of thing
whatever feed JDS gets from Bitcoin Core (be it updates with single or batched txs), it should contain full `txdata` instead of only `wtxids`
then it's up to JDS implementation to aim to optimize which txs it could selectively drop to avoid unbounded memory consumption
but `txdata` should be made available sooner, rather than selectively fetched (with extr
...
π¬ rkrux commented on pull request "psbt: detect invalid MuSig2 pubkeys in deserialization":
(https://github.com/bitcoin/bitcoin/pull/34010#issuecomment-3627216261)
> I suspect a better place to fix this would be DeserializeMuSig2ParticipantPubkeys, where a validity check is currently missing.
Updated PR to add the check there instead of in the MuSig2 signing flow that occurs later.
> Could you add a simple test for this?
Added few tests from the fuzz inputs and created one more test so that the test coverage of an existing `if` condition that appears at the end of `DeserializeMuSig2ParticipantPubkeys` is not lost due to the newly added check for
...
(https://github.com/bitcoin/bitcoin/pull/34010#issuecomment-3627216261)
> I suspect a better place to fix this would be DeserializeMuSig2ParticipantPubkeys, where a validity check is currently missing.
Updated PR to add the check there instead of in the MuSig2 signing flow that occurs later.
> Could you add a simple test for this?
Added few tests from the fuzz inputs and created one more test so that the test coverage of an existing `if` condition that appears at the end of `DeserializeMuSig2ParticipantPubkeys` is not lost due to the newly added check for
...
π¬ Sjors commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209)
In fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_:
I think the original intention here was to demonstrate that `generate` interrupts an ongoing `waitNext()`.
Now it only shows that `waitNext()` returns immediately if the tip has changes before calling. Though perhaps it always did that by accident.
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209)
In fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_:
I think the original intention here was to demonstrate that `generate` interrupts an ongoing `waitNext()`.
Now it only shows that `waitNext()` returns immediately if the tip has changes before calling. Though perhaps it always did that by accident.
π¬ Sjors commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386)
In https://github.com/bitcoin/bitcoin/commit/fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_: similar to my comment above, this only shows that `waitNext()` returns immediately if fees have gone up before the call, but the original intent was to show that a fee increase causes the wait to be interrupted.
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386)
In https://github.com/bitcoin/bitcoin/commit/fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_: similar to my comment above, this only shows that `waitNext()` returns immediately if fees have gone up before the call, but the original intent was to show that a fee increase causes the wait to be interrupted.
π¬ Sjors commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598816528)
In fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_: `await stack.enter_async_context(destroying((` is hard to read, so maybe move it into a helper:
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 3d28bba136..e01c753d24 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -96,6 +96,14 @@ class IPCInterfaceTest(BitcoinTestFramework):
tx.deserialize(coinbase
...
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598816528)
In fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb _test: fix interface_ipc.py template destruction_: `await stack.enter_async_context(destroying((` is hard to read, so maybe move it into a helper:
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 3d28bba136..e01c753d24 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -96,6 +96,14 @@ class IPCInterfaceTest(BitcoinTestFramework):
tx.deserialize(coinbase
...
π¬ ryanofsky commented on issue "consider adding a new `interface RawTxFeed` on Mining IPC":
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627326527)
> We haven't used IPC for ZMQ-style streaming yet. In the Mining interface you call a method and get a result, immediately or after a delay. So this might be quite involved.
This is true for the mining interface, but the other interfaces in #29409 and #10102 do used streamed notifications. The wallet uses `Chain.handleNotifications` to start receiving notifications and transactions and blocks:
https://github.com/bitcoin/bitcoin/blob/d9efd1e49d1df154970b6a60229eedde3ba7cffe/src/ipc/capnp/chain.
...
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627326527)
> We haven't used IPC for ZMQ-style streaming yet. In the Mining interface you call a method and get a result, immediately or after a delay. So this might be quite involved.
This is true for the mining interface, but the other interfaces in #29409 and #10102 do used streamed notifications. The wallet uses `Chain.handleNotifications` to start receiving notifications and transactions and blocks:
https://github.com/bitcoin/bitcoin/blob/d9efd1e49d1df154970b6a60229eedde3ba7cffe/src/ipc/capnp/chain.
...
π¬ adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3627356662)
I've addressed all nits and rebased. Thank you to each of you who left thoughtful commentary.
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3627356662)
I've addressed all nits and rebased. Thank you to each of you who left thoughtful commentary.
π¬ fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3627362057)
I've pulled in a commit from @laanwj that is a partial reversion of https://github.com/gcc-mirror/gcc/commit/1d82fc2e6824bf83159389729c31a942f7b91b04.
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3627362057)
I've pulled in a commit from @laanwj that is a partial reversion of https://github.com/gcc-mirror/gcc/commit/1d82fc2e6824bf83159389729c31a942f7b91b04.
π¬ sipa commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-3627367982)
Weak Concept NACK.
Given how broken the examples are, I think we should fix them throughout and add testing to prevent regressions, or get rid of them entirely.
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-3627367982)
Weak Concept NACK.
Given how broken the examples are, I think we should fix them throughout and add testing to prevent regressions, or get rid of them entirely.
π pablomartin4btc approved a pull request: "netbase: Remove "tor" as a network specification"
(https://github.com/bitcoin/bitcoin/pull/34031#pullrequestreview-3552560740)
ACK 7efb18c38150344ca6f7efbcd8441792a2ea921f
(https://github.com/bitcoin/bitcoin/pull/34031#pullrequestreview-3552560740)
ACK 7efb18c38150344ca6f7efbcd8441792a2ea921f
π¬ Sjors commented on issue "consider adding a new `interface RawTxFeed` on Mining IPC":
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627477785)
A pool's JDS processes proposed templates from multiple miners, which will have divergent mempools. So until a block is mined, some templates may still have transaction that the pool itself threw out of its mempool.
In order for the JDS to prune its pseudo-mempool it could track which inputs are spent in the new block and (recursively) delete entries that spend from it.
(https://github.com/bitcoin/bitcoin/issues/34030#issuecomment-3627477785)
A pool's JDS processes proposed templates from multiple miners, which will have divergent mempools. So until a block is mined, some templates may still have transaction that the pool itself threw out of its mempool.
In order for the JDS to prune its pseudo-mempool it could track which inputs are spent in the new block and (recursively) delete entries that spend from it.
π¬ darosior commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599070189)
Should we explicitly document "this function will not check pow!" then?
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599070189)
Should we explicitly document "this function will not check pow!" then?
π¬ sipa commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2599076399)
It's a bit strange to use an imperative form in release notes, given that it's describing changes that have been made already.
Suggestion:
> CLI `-addrinfo` now returns the full set of known addresses. In previous versions (v22.0 - v30.0) the set of returned addresses was filtered for quality and recency. This was changed since it does not match the logic for selecting peers to connect to, which does not filter.
---
Also worth mentioning that the CLI feature is now imcompatible with
...
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2599076399)
It's a bit strange to use an imperative form in release notes, given that it's describing changes that have been made already.
Suggestion:
> CLI `-addrinfo` now returns the full set of known addresses. In previous versions (v22.0 - v30.0) the set of returned addresses was filtered for quality and recency. This was changed since it does not match the logic for selecting peers to connect to, which does not filter.
---
Also worth mentioning that the CLI feature is now imcompatible with
...