Bitcoin Core Github
44 subscribers
119K links
Download Telegram
👍 hodlinator approved a pull request: "ci: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3279511439)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f
💬 hodlinator commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387762653)
I guess the amount of Mac-specific code we have that behaves differently between 14 and 15 is minimal.

If it hasn't been a problem previously I'm okay with bumping it so we don't need to re-touch in 1 year - Although if we are going to keep bumping the minimum supported version and CI version every year anyway... I would lean towards keeping them in sync. (GitHub deprecating and removing older versions would then also serve as an extra reminder if we are late to update).
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-3346773372)
`0cd86a5c70...8218daac0f`: rebase due to conflicts
💬 vasild commented on pull request "i2p: make a time gap between creating transient sessions and using them":
(https://github.com/bitcoin/bitcoin/pull/32065#issuecomment-3346791115)
`632eb1a3c9...a42449cb30`: rebase due to conflicts
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-3346806725)
`22d4c57a99...9a49877f26`: rebase due to conflicts
💬 maflcko commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2387881324)
If the version is too low here, backport branches would have to bump it as well at some point. But either way should be equally fine. Happy to push any version, but for now I'll leave as-is to not invalidate the two acks.
👍 vasild approved a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180#pullrequestreview-3279732989)
ACK b2c6cb7f026ff704c227e632f8c5c2f09e6f058a
📝 CezaryNiklewicz opened a pull request: "Create szwed"
(https://github.com/bitcoin/bitcoin/pull/33493)
<!--
*** 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 improvements or new tests that improv
...
💬 CezaryNiklewicz commented on pull request "Create szwed":
(https://github.com/bitcoin/bitcoin/pull/33493#issuecomment-3346891701)
menelica
💬 CezaryNiklewicz commented on pull request "Create szwed":
(https://github.com/bitcoin/bitcoin/pull/33493#issuecomment-3346892130)
sss
💬 Raimo33 commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3346913393)
reACK d870caca33ac9f0fdd76969a7341535c5722417e
🤔 rkrux reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3279715340)
Code review complete at 36f83554a2dc5b397ee3b4495d8bf2aff36cedc6.

I will share my final thoughts in a subsequent comment.
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2387893415)
In 793d727791fe042bbb4a9ff5df3db012a77a40fe "sign: Add CreateMuSig2Nonce"

Wouldn't hurt to add this assert imo. I'm inclining to fail early the whole process in case of any such scenarios.

```diff
diff --git a/src/key.cpp b/src/key.cpp
index a952acb260..f7e065c8da 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -372,6 +372,8 @@ std::vector<uint8_t> CKey::CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uin
if (!secp256k1_musig_nonce_gen(secp256k1_context_sign, secnonce.Get(), &p
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2387925458)
In 2806f8152c751641257f3f1ab0d886d9c8ece1e1 "pubkey: Return tweaks from BIP32 derivation"

Reconsider updating the commit description retouched, I can foresee future readers would want to know the intent of this commit, following up on https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3282694791
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2387919138)
In 36f83554a2dc5b397ee3b4495d8bf2aff36cedc6 "test: Test MuSig2 in the wallet"

I feel it'd be prudent to be comprehensive in testing here. A more practical scenario would be where the `MuSig2` unspents are combined with non-MuSig2/Taproot unspents, and/or there could be multiple MuSig2 unspents in the PSBT. The following diff tests for:
1. 2 MuSig2 inputs in the PSBT along with 1 SegWit unspent. A secondary reason is to avoid seeing `dec_psbt["inputs"][0]` in several places.
2. Iterates all
...
🤔 janb84 reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3279795119)
reACK d870caca33ac9f0fdd76969a7341535c5722417e
📝 hebasto opened a pull request: "depends: Update URL for `qrencode` package source tarball"
(https://github.com/bitcoin/bitcoin/pull/33494)
The https://fukuchi.org/ homepage no longer links to the source tarball, and previously available files appear to have been removed. The homepage now instructs users to download source tarballs from the GitHub [releases](https://github.com/fukuchi/libqrencode/releases) page instead.

The diff between the source trees is immaterial:
```diff
--- old
+++ new
@@ -1,19 +1,16 @@
27e7deccd2925c94e4190ee64794a051199f215f145f76fd664cdebedbbf8a35 acinclude.m4
-e1e35b1309482f699a9700a2065a0bce09c
...
📝 JaroslawDomagala opened a pull request: "Create masteron"
(https://github.com/bitcoin/bitcoin/pull/33495)
<!--
*** 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 improvements or new tests that improv
...
💬 JaroslawDomagala commented on pull request "Create masteron":
(https://github.com/bitcoin/bitcoin/pull/33495#issuecomment-3347001048)
22
👍 vasild approved a pull request: "Introduce SockMan ("lite"): low-level socket handling for HTTP"
(https://github.com/bitcoin/bitcoin/pull/32747#pullrequestreview-3279884844)
ACK e007e1b57d5d42c2a8d932d5b91eec8a3ca76e14