Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598302815)
> `TipChainstate`

Not sure this is clearer. All chainstates have a tip, each. So it would be unclear, which tip-chainstate this refers to :sweat_smile:
πŸ’¬ maflcko commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r2598307984)
> It's not validated until later and the maybe refers to `std::optional` afaict.

I think the name is fine, because the stats (if they exist), are validated, because they are derived from the validated chainstate. But not strong opinion.
πŸ’¬ vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2597405891)
This is somewhat of a misuse of `p2p_port()`. It works, but looking at it I wonder if it can be done better. Maybe introduce `proxy_port(n)`? At some point if we implement also SAM proxy (for hijacking I2P connections), then we might need e.g. `proxy_port(0)` for the SOCKS5 proxy and `proxy_port(1)` for the SAM proxy.
πŸ’¬ vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2597394343)
Would it make sense to introduce `auto_listener_port(n)`?
πŸ’¬ vasild commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2598416420)
If this is changed to:

```diff
- # Create listener with default P2PInterface
- listener = P2PInterface()
- actual_to_addr, actual_to_port = self._create_auto_outbound_listener(listener, i)
+ if self.decide_auto_outbound_where_to:
+ actual_to_addr, actual_to_port, listener = self.decide_auto_outbound_where_to(i)
+ else:
+ # Create listener with default P2PInterface
+
...
πŸ’¬ adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2598446881)
switched to tx number (1-based indexing) for user reporting
πŸ’¬ 0xB10C commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3626793175)
- rebased to include https://github.com/bitcoin/bitcoin/pull/29641. Now using `LogInfo` instead of `LogPrintf`. Updated the commit message an PR description accordingly.
- Switched to AJ's suggestion from https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2596296831 to only have minimal code duplication. This logs all new outbound connections with `LogInfo()`, which can potentially get rate-limited, and logs inbound connections with `LogDebug(BCLog::NET, ...)` which is only active with `
...
πŸ‘‹ 0xB10C's pull request is ready for review: "log: don't rate-limit "new peer" with -debug=net"
(https://github.com/bitcoin/bitcoin/pull/34008)
πŸ“ albert4719 opened a pull request: "Improve Chinese translations for peer connection types"
(https://github.com/bitcoin-core/gui/pull/917)
This PR improves several Chinese translations related to peer connection types in the
Bitcoin Core Qt GUI. The existing translations contain a mix of literal interpretations
and colloquial wording that can be misleading or unclear in a technical networking UI.

Specifically:

- "Manual" was translated as "ζ‰‹ε†Œ", which commonly refers to documentation in Chinese.
In Bitcoin Core, "Manual" indicates a peer connection that was manually established.
This PR updates the translation to "ζ‰‹εŠ¨θΏžζŽ₯
...
πŸ’¬ fanquake commented on pull request "Improve Chinese translations for peer connection types":
(https://github.com/bitcoin-core/gui/pull/917#issuecomment-3626809753)
Thanks, however translations are handled via Transifex. Please contribute any changes there.
βœ… fanquake closed a pull request: "Improve Chinese translations for peer connection types"
(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`.
πŸ’¬ 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.
πŸ’¬ 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:
πŸ’¬ 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
πŸ’¬ 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
...
πŸ’¬ 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
πŸ’¬ 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
...
πŸ’¬ 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
...
πŸ’¬ 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.