Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 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_r2333720544)
In https://github.com/bitcoin/bitcoin/commit/d23116987d19587746d392895c06f5c426c1d0d2 "test: Test MuSig2 in the wallet"

With the addition of the case where musig is both in keypath and spendpath, I think it would be nice to assert which spending path is triggered.

<details open>
<summary>Diff</summary>

```diff
diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py
index b7f3cc9d96..63276b1eb7 100755
--- a/test/functional/wallet_musig.py
+++ b/test/functional
...
💬 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_r2333473293)
In d23116987d19587746d392895c06f5c426c1d0d2 "test: Test MuSig2 in the wallet"

Can also add the below test case that works right away where the musig is in both the key path spend and the script path spend - KP has all 3 keys in the musig, SP scripts have 2 partial keys in their musig each.

```python
self.do_test("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})}", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})")
```
💬 instagibbs commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3270872088)
Is there a easy to read table somewhere on how you are interpreting and using specific error codes / msgs and how your software is intended to respond to them? This seems like an twice a year issue, and as far as I know no one else is using the errors for non-debugging reasons, or they're too shy to complain about the breaks.
🤔 sipa reviewed a pull request: "multiprocess: Don't require bitcoin -m argument when IPC options are used"
(https://github.com/bitcoin/bitcoin/pull/33229#pullrequestreview-3201821957)
utACK f9685d6a1389938b0cceb31d9eef201ab3dd2e86
💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3270914430)
Updated the description and added this as a 30.0 miestone in case that makes sense. I'm not exactly sure how release milestones are used so we can remove it if this is inappropriate
💬 sipa commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2333792206)
This `INTERNET_TRAFFIC_EXPECTED` seems to be unused?
💬 ziggie1984 commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3270947536)
We don't have an easy table but I think for example this gives a good example:

https://github.com/lightningnetwork/lnd/blob/master/sweep/fee_bumper.go#L568-L578

In our fee bumper engine we only bump the fee of a transaction if the error is somehow related to an `insufficient fee` error, otherwise we drop this input from the sweeping system because problems like not having the correct signature or being already spent would cause us to never properly sweep this input.

What I am more interested
...
🤔 stickies-v reviewed a pull request: "chore: compare against keys.size() in writeObject comma check"
(https://github.com/bitcoin/bitcoin/pull/33337#pullrequestreview-3201895990)
NACK, seems like busywork, no clear improvement
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2333831377)
Note that the only musig API function that requires to use this context is [`secp256k1_musig_nonce_gen`](https://github.com/bitcoin-core/secp256k1/blob/36e76952cbf1cf54ddd2d8756cc31a486e2ba1d9/include/secp256k1_musig.h#L338), for all the others you can simply use the static one (`secp256k1_context_static`).
fanquake closed a pull request: "chore: compare against keys.size() in writeObject comma check"
(https://github.com/bitcoin/bitcoin/pull/33337)
💬 sipa commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2333834031)
I think just setting the tip to 0 is fine. We don't actually know whether any of the tip's ancestors were activatable earlier than any of their potential competitors.
💬 instagibbs commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3270998612)
Speaking only for myself since I'm more familiar with both LN (by means of CLN) and Bitcoin Core sides...

> Yes I am really surprised that people are not using those error codes to decided based on that what is wrong with their transaction what they for example trying to broadcast.

I think it's honestly quite rare. People will craft transactions using information gleaned Core (like mempool minfee or fee estimator) and proactively avoid violating it. The software can't do anything about other e
...
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3271072287)
> `getblockstats` gives the same result for both at height 913,859

Did you mean `gettxoutsetinfo` here? :)
🤔 sipa reviewed a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-3202025791)
utACK 0465574c127907df9b764055a585e8281bae8d1d

Happy with either the "whole active chain is set to nSequence 0" or the "only tip is set to nSequence 0" approaches.
🤔 sipa reviewed a pull request: "headerssync: Correct unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3202085490)
utACK 33d550d3044f9075cc866093c453158288f12dec
💬 sipa commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2333959308)
In commit "refactor(headerssync): Process spans of headers"

Nit: slightly simpler: `std::span{first_chain}.subspan(1)`.
💬 sipa commented on pull request "headerssync: Correct unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2333960814)
In commit "refactor(headerssync): Process spans of headers"

Same here: `std::span{second_chain}.subspan(1)`.
💬 mzumsande commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3271246212)
> My understanding of [`Peer::TxRelay::m_tx_inventory_known_filter`](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/net_processing.cpp#L297-L300) is that it will [occasionally indicate](https://github.com/bitcoin/bitcoin/blob/e04cb9c1bdf2199127cc8cf9c87f25e46b8cac7b/src/common/bloom.h#L91-L92) that a transaction is known to a peer when it isn't, and then we won't include it in an INV:

Good point, I didn't think of that possibility.

> Maybe this is also
...
💬 ziggie1984 commented on issue "Difficulty in reliably mapping errors from Bitcoin Core due to unstable error codes and messages":
(https://github.com/bitcoin/bitcoin/issues/33350#issuecomment-3271254766)
Ok thank you for the information so far, feels like there is currently no easy way for us to proceed so we continue mapping the error strings. What would be very helpful is like a release-note highlight in case the error reporting for the RPC changes.

Keeping this issue open for a bit in case somebody else has a better idea, thank you so far for your thoughts.
💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3271268644)
@theuni switched this to using `CMAKE_EXE_LINKER_FLAGS`, which should make it clearer which flags are being used for executables.