Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 w0xlt opened a pull request: "[rpc] `gettxout`: array argument for bulk queries"
(https://github.com/bitcoin/bitcoin/pull/33531)
`gettxout` supports only a single outpoint per call, requiring multiple RPC round trips to inspect all inputs of a transaction.

This PR changes gettxout to accept an array of {txid, vout} objects, processing them in a single pass and returning results aligned to the inputs.

This reduces latency, avoids redundant lookups, and improves throughput for batch use cases (e.g. wallets, indexers, transaction verification).

For simplicity, the changes are not backward compatible, but this can b
...
💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#issuecomment-3363772867)
Added kernel notifications (thanks @ryanofsky) and improved crash resistance at the very last step (final rename back to old names) - try it out with `./build/bin/bitcoin-qt -reobfuscate-blocks -stopatheight=1`.
<img width=500 src="https://github.com/user-attachments/assets/95b72e06-63fe-4057-989d-24ab9e93bfac" />
👋 l0rinc's pull request is ready for review: "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup"
(https://github.com/bitcoin/bitcoin/pull/33324)
🤔 yuvicc reviewed a pull request: "log: print every script verification state change"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3297532271)
ACK 45bd8914658a675d00aa9c83373d6903a8a9ece8

Reviewed all commits.
💬 stratospher commented on pull request "p2p: Use network-dependent timers for inbound inv scheduling":
(https://github.com/bitcoin/bitcoin/pull/33464#issuecomment-3364509891)
reACK 0f7d4ee.

> Removed the atomic and guarded m_next_inv_to_inbounds_per_network_key by g_msgproc_mutex.

didn't notice any change in the rate at which `m_next_inv_send_time = NextInvToInbounds(..)` logs were printed after removing the atomic + no locking related calls in the assembly dump. would be curious as well to know what others think of [this diff](https://github.com/bitcoin/bitcoin/compare/beb75e48ae1d5771932f427a490c7e1b6c1720d3..0f7d4ee4e8281ed141a6ebb7e0edee7b864e4dcf).
👍 vasild approved a pull request: "Improve LastCommonAncestor performance + add tests"
(https://github.com/bitcoin/bitcoin/pull/33515#pullrequestreview-3298109883)
ACK 3635d62f5a935801e26a0d5fa2cb5e2dbbb42f9b
👍 vasild approved a pull request: "depends: Update URL for `qrencode` package source tarball"
(https://github.com/bitcoin/bitcoin/pull/33494#pullrequestreview-3298300084)
ACK 93a70a42d30fa2f9404b76d5bbdb5ea316fc1032
💬 luke-jr commented on pull request "[rpc] `gettxout`: array argument for bulk queries":
(https://github.com/bitcoin/bitcoin/pull/33531#issuecomment-3365140035)
Concept NACK, just use JSON-RPC batching
💬 luke-jr commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33474#issuecomment-3365167848)
ACK 2d7ebd2d913ea63c1a23fefa0a09ee06fb069161
💬 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
...