Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3365168707)
Thanks @davidgumberg!

@hebasto should have addressed your comments.
🤔 stickies-v reviewed a pull request: "[rpc] `gettxout`: array argument for bulk queries"
(https://github.com/bitcoin/bitcoin/pull/33531#pullrequestreview-3298359050)
I think this PR needs much better motivation, especially with the breaking changes:
1) what are the observed performance differences between:
- master and this PR for non-batch query
- master and this PR for batch query
2) has this been a problem / request by any downstream users? If so, would be useful to document here.

> For simplicity, the changes are not backward compatible, but this can be adjusted based on review feedback.

If the backwards incompatibility is kept, it definite
...
💬 stickies-v commented on pull request "[rpc] `gettxout`: array argument for bulk queries":
(https://github.com/bitcoin/bitcoin/pull/33531#discussion_r2401457753)
This can be cleaned up with the `Arg` helper, making the code more concise and removing the duplication of the default value definition:

<details>
<summary>git diff on b361b45707</summary>

```diff
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 63bd172e52..74631a33f6 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1206,9 +1206,7 @@ static RPCHelpMan gettxout()
Chainstate& active_chainstate = chainman.ActiveChainstate();
CCoinsViewCa
...
💬 stickies-v commented on pull request "[rpc] `gettxout`: array argument for bulk queries":
(https://github.com/bitcoin/bitcoin/pull/33531#discussion_r2401488034)
`gettxspendingprevout` also takes an array of txid/vout, perhaps worth carving that out into helper functions and reusing it here? (there might be even more users of this logic, didn't do an exhaustive search)
💬 stratospher commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#issuecomment-3365240416)
ACK 14ae71f. nice restructuring using mocktime!
🤔 rkrux requested changes to a pull request: "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key"
(https://github.com/bitcoin/bitcoin/pull/32471#pullrequestreview-3298207486)
Requesting changes because found an issue with the `musig()` descriptor that can be fixed in this PR itself.
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2401362264)
In a797b2ab6ba69732a1b6f1b54b7785d4b802170e "walletrpc: reject listdes with priv key on w-only wallets"

Nit: To make it explicit in the error that the request params combination is invalid.
```diff
- throw JSONRPCError(RPC_WALLET_ERROR, "Can't get private descriptor string for watch-only wallets");
+ throw JSONRPCError(RPC_WALLET_ERROR, "Invalid request to get private descriptor string for watch-only wallets");
```
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2401352937)
In 74521dd6d4b65007547e64bda2bfbc6fe497677f "test: Test listdescs with priv works even with missing priv keys"

Consider importing few musig descriptors as well. Noticed this was an issue while reviewing #29675: https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2330079926

Plus minor refactor to use `set` for importing and asserting to make it less redundant to read.

<details open>
<summary>More musig tests and refactoring diff</summary>

```diff
diff --git a/test/functional/w
...
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2401425187)
There appears to be an issue with the following MuSig descriptor that fails the test:

One of the MuSig portion in the descriptor doesn't contain any private key for which the `listdescriptors(true)` returns a `pk()` instead of defaulting to the public keys whereas the `listdescriptors()` works fine.

```python
desc = descsum_create('tr(03dff1d77f2a671c5f36183726db2341be58feae1da2deced843240f7b502ba659,{pk(musig(tpubD6NzVbkrYhZ4Wo2WcFSgSqRD9QWkGxddo6WSqsVBx7uQ8QEtM7WncKDRjhFEexK119N
...
💬 maflcko commented on pull request "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0":
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-3365320409)
Is this still needed after https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2729341917 ?
💬 janb84 commented on pull request "guix: use LIEF 0.17.0":
(https://github.com/bitcoin/bitcoin/pull/33522#issuecomment-3365360108)
This work for my guix build

<details><summary>Guix Build Output</summary>

**Host architecture:** `aarch64`
**Commit:** `0123c5f56c20`

```shell
f83387eefc9384d17a6986dc5390f2f45cc30391af559dbb0dfa7975e3bab6c1 guix-build-0123c5f56c20/output/aarch64-linux-gnu/SHA256SUMS.part
336c4f647e5318d34119580377f7bb957fe4cc2009160712f14b04f5d6876386 guix-build-0123c5f56c20/output/aarch64-linux-gnu/bitcoin-0123c5f56c20-aarch64-linux-gnu-debug.tar.gz
8f6d8197eec76f8bf20ad936dde15e06cfa5b69110
...
💬 instagibbs commented on issue "compact blocks in IBD resets m_stalling_since":
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-3365361040)
> Lastly, the CanDirectFetch check is interesting. It will fail if our tip time is not within 3h20m of our local clock.

Yeah this is a known weirdness @dergoegge . I don't think there's any real motivation to do it, defectors can "reset" the MTP forward IIUC. Extremely rare blocks or a big drop in hashing could cause it, but in the latter case lower block rate also makes races less likely.

I'll spend some time next week hopefully trying to mentally tackle this.
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#discussion_r2401609688)
`s/HavePrivateKeys/HasAllPrivateKeys` because the current implementation of `HavePrivateKeys` returns `false` immediately if a corresponding private key is not found for a public key; the function doc suggests this too.
> Whether the given provider has all private keys required by this descriptor.

Whereas `ToPrivateString` returns `true` if any private key is found, the usage of `any_success` also suggests this.
💬 naiyoma commented on pull request "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value":
(https://github.com/bitcoin/bitcoin/pull/33498#discussion_r2401632810)
Yes, this is more precise, ive updated
⚠️ DevRatnu opened an issue: "version 30.0 as tested 2.9.1.0"
(https://github.com/bitcoin/bitcoin/issues/33532)
### Motivation

Bitcoin OP_return function already tested into 2.9.1.0 Version and 30.0 is ready

### Possible solution

Bitcoin OP_return function already tested into 2.9.1.0 Version and 30.0 is ready check this - https://github.com/virtacoin/VirtaCoinProject

### Useful Skills

* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...


### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, please read [CONTR
...
pinheadmz closed an issue: "version 30.0 as tested 2.9.1.0"
(https://github.com/bitcoin/bitcoin/issues/33532)
👋 fanquake's pull request is ready for review: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33474)
💬 yuvicc commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3365615151)
Rebased to https://github.com/bitcoin/bitcoin/commit/1ed00a0d39d5190d8ad88a0dd705a09b56d987aa.
🤔 hebasto reviewed a pull request: "guix: use LIEF 0.17.0"
(https://github.com/bitcoin/bitcoin/pull/33522#pullrequestreview-3298906597)
If no bugs are fixed and no new features are needed, perhaps we should do the opposite and wait until the Guix commit used for release builds is updated to include 0.16.6 as a [package](https://codeberg.org/guix/guix/commit/b99a2f21d1335834e893109d10b55c43bb9f5562)?
💬 instagibbs commented on pull request "Mempool: Do not enforce TRUC checks on reorg":
(https://github.com/bitcoin/bitcoin/pull/33504#issuecomment-3365641140)
I'll open a follow-up soon thanks for the in depth review here