Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 pablomartin4btc opened a pull request: "rpc, test: Fix JSON parsing errors in RPC calls (unloadwallet and getdescriptoractivity) raising RPC_INVALID_PARAMETER with appropriate description"
(https://github.com/bitcoin/bitcoin/pull/32845)
Currently, `unloadwallet` RPC call fails with a JSON parsing error when no `wallet_name` argument is provided. This behavior is misleading because the error originates from a low-level JSON type mismatch, rather than clearly indicating that the wallet name or RPC endpoint (`-rpcwallet=...`) is missing. Also, found out that the [issue](https://github.com/bitcoin/bitcoin/pull/13111#issuecomment-398831543) was noticed during its implementation but never addressed.

In addition, I've verified all
...
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2176170471)
Fixed, thanks.
💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176171672)
Is the cause of the mismatch that you thought the scriptPubKey directly contains the wtxid tree root?
It actually contains `Double-SHA256(witness root hash|witness reserved value)`
So you also have to extract the `witness reserved value` from the coinbase input witness.
see https://github.com/bitcoin/bips/blob/77b0bb297ad4e4e6b04b2c078c5e3b620a7090e2/bip-0141.mediawiki#commitment-structure
💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021260691)
> Feature requested by @SomberNight for Electrum's Lightning stuff. (Please confirm this does what you need)

Wow, thank you for working on this! I was meaning to open an issue to ask for this, but did not even get to that yet.

The request I would like an electrum server, assuming txindex=1, to be able to serve is:

`bc.tx.get_merkle_witness(txid)`, (so given just a `txid`,)

return a dict containing:
- `wtxid`
- `block_height`
- `block_hash`
- `pos` (index of tx in block)
- `mer
...
💬 SomberNight commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176194870)
My cpp is beyond rusty, so maybe I misunderstand, but note that just because the tx to be proved does not have a witness, we would still want a witness-SPV proof for that tx.

The attack vector is that a malicious electrum server can notify the client about a tx touching their address and strip away the witness. That is, the actual tx has a witness, but the server sends it to the client using the old serialization format without it. Then when the client asks for the SPV proof, the server (stil
...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176203977)
so, something like this?

`// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
return {};
}
const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitness.stack[0];
if (witness_nonce.size() != 32) {
return {};
}

// Calculate the witness
...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021345326)
@ghost43 hmm.. like this?

Here's the fix needed in `src/rpc/txoutproof.cpp` in the `verifytxoutproof` function after line 220:

```cpp
const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);

// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
return {};
}
const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitness.stack[0];
if (witness_nonce.size() !=
...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021350135)
@SomberNight hmm.. like this?

Here's the fix needed in `src/rpc/txoutproof.cpp` in the `verifytxoutproof` function after line 220:

```cpp
const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);

// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
return {};
}
const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitness.stack[0];
if (witness_nonce.size(
...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021358317)
i'm fairly new to this... let's see if that's correct.
💬 achow101 commented on pull request "doc: Add fetching single PRs from upstream to productivity.md":
(https://github.com/bitcoin/bitcoin/pull/32783#issuecomment-3021366309)
ACK 45b1d39757668939b03b27401c324a938ef0cd8d
🚀 achow101 merged a pull request: "doc: Add fetching single PRs from upstream to productivity.md"
(https://github.com/bitcoin/bitcoin/pull/32783)
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021534956)
> @SomberNight hmm.. like this?
>
> Here's the fix needed in `src/rpc/txoutproof.cpp` in the `verifytxoutproof` function after line 220:
>
> ```c++
> const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);
>
> // Extract witness nonce from coinbase input
> if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
> return {};
> }
> const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitnes
...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#discussion_r2176303594)
@SomberNight hmm.. like this?

Here's the fix needed in `src/rpc/txoutproof.cpp` in the `verifytxoutproof` function after line 220:

```cpp
const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);

// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_gentx->vin[0].scriptWitness.stack.empty()) {
return {};
}
const auto& witness_nonce = merkleBlock.m_gentx->vin[0].scriptWitness.stack[0];
if (witness
...
🤔 ryanofsky reviewed a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345#pullrequestreview-2973346180)
Rebased 3ed3f7f979fb8e4cf3023deb64acf9b0a545057a -> 5024f552924b214fa8a092c1daf9acf92a4c40f0 ([`pr/ipc-stop.11`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.11) -> [`pr/ipc-stop.12`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.12), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.11-rebase..pr/ipc-stop.12)) with update to fix tidy errors and try to incorporate https://github.com/bitcoin-core/libmultiprocess/pull/186

Will mark as a draft since this n
...
💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2176329943)
re: https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2171026682

> CI on [Sjors#90 (comment)](https://github.com/Sjors/bitcoin/pull/90#issuecomment-3011857912) complains of an unused result. Which is odd because the `(void)` cast should prevent such a warning. This seems to be a gcc thing: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425

Thanks! I tried to work around this by adding a `!` though reading gcc issue maybe that will not be sufficient.
💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2176327196)
re: https://github.com/bitcoin/bitcoin/pull/32345#discussion_r2171082503

> `<csignal>`

Thanks, fixed
📝 ryanofsky converted_to_draft a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345)
This PR fixes various problems when IPC connections are broken or hang which were reported in https://github.com/bitcoin-core/libmultiprocess/issues/123, https://github.com/bitcoin-core/libmultiprocess/issues/176, and https://github.com/bitcoin-core/libmultiprocess/pull/182. The different fixes are described in commit messages.

---

The first two commits of this PR update the libmultiprocess subtree including the following PRs:

- https://github.com/bitcoin-core/libmultiprocess/pull/181

...
💬 bigshiny90 commented on pull request "RPC/txoutproof: Support including (and verifying) proofs of wtxid":
(https://github.com/bitcoin/bitcoin/pull/32844#issuecomment-3021687570)
@SomberNight

Compilation of potential fixes for PR #32844:

## 1. Witness nonce issue
The witness commitment wasn't being calculated correctly. Added in `src/rpc/txoutproof.cpp`:

First, add required includes at the top:
```cpp
#include <hash.h>
#include <logging.h>
```

Then after line 220:

```cpp
const auto wtxid_root = merkleBlock.m_wtxid_tree.ExtractMatches(wtxid_matches, wtxid_indexes);

// Extract witness nonce from coinbase input
if (merkleBlock.m_gentx->vin.empty() || merkleBlock.m_ge
...
🤔 yuvicc reviewed a pull request: "test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()"
(https://github.com/bitcoin/bitcoin/pull/32823#pullrequestreview-2973475932)
ACK ec004cdb86e6471915e1033f390c76ee0428e415

- some comments has been updated to maintain readability
- update `wait_for_getheader` to use `rehash()` for robust testing
💬 maflcko commented on pull request "doc: add `/spenttxouts` to REST-interface.md":
(https://github.com/bitcoin/bitcoin/pull/32842#issuecomment-3021867622)
lgtm ACK dd99cedc0bfe7d7eee0f543bb27dab005c426c66