💬 jonatack commented on pull request "init: Configure reachable networks before we start the RPC server":
(https://github.com/bitcoin/bitcoin/pull/32539#issuecomment-2945122017)
Concept ACK, will review

🤔 pablomartin4btc reviewed a pull request: "test: wallet: cover wallet passphrase with a null char"
(https://github.com/bitcoin/bitcoin/pull/32675#pullrequestreview-2900967011)
cr ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064

In the description: _"This PR adds test coverage for the `walletpassphrase` RPC..."_ should include also _"`walletpassphrasechange`"_.

💬 pablomartin4btc commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2129224585)
nit: again if you have to retouch... otherwise when you search for that "similar" description in the code you get both places (_"The wallet"_ and _"The Old wallet"_)
```suggestion
assert_raises_rpc_error(-14, "The wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrase, passphrase_with_nulls + "\0", 10)
```

💬 pablomartin4btc commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2129224684)
nit: if you have to retouch maybe... even if it's not entirely part of the scope of the change...
```suggestion
# walletpassphrasechange should not stop at null characters as prior to v25.0
```
And a test could be added in `wallet_backcompatibility.py` if it makes sense.

💬 pablomartin4btc commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2129230799)
nit: just if you retouch... to be consistent with other places where that error description is being compared...
```suggestion
assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrasechange, passphrase_with_nulls.partition("\0")[0], "abc")
```

💬 Muniru0 commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/32683#issuecomment-2945206230)
Hi @Fuzion24,

I have not seen an existing issue filed for this yet, but I believe it reflects a known edge case in Bitcoin Core.


### 🔍 Root Cause
This error occurs when the wallet fails to estimate transaction fees for non-solvable inputs, such as those found in watch-only wallets or descriptors lacking private keys. The walletcreatefundedpsbt RPC is designed to select inputs and estimate fees, but when it encounters inputs it cannot sign, it lacks sufficient information (e.g., signature siz
...

💬 Fuzion24 commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/32683#issuecomment-2945223480)
I had previously tried with an explicit feeRate, but here's exactly your example and it's output:

```
walletcreatefundedpsbt "[]" '{"<redacted address>":0.1}' 0 '{"feeRate":0.0001}'
error code: -4
error message:
Internal bug detected: Fee needed > fee paid
./wallet/spend.cpp:1320 (CreateTransactionInternal)
Bitcoin Core v29.0.0
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
```

💬 maflcko commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/32683#issuecomment-2945263901)
@Muniru0 Please do not post LLM generated blobs blindly. As can be seen, this isn't helpful.


Similar report: https://github.com/bitcoin/bitcoin/issues/29398

@Fuzion24 It would be helpful if the issue could be reproduced step-by-step on a freshly created regtest testing wallet.

👍 hebasto approved a pull request: "deps: Bump lief to 0.16.6"
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2901133490)
ACK 74dfa4155037306d79596521a5c38a9e90487331.

💬 Muniru0 commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/32683#issuecomment-2945416828)
> [@Muniru0](https://github.com/Muniru0) Please do not post LLM generated blobs blindly. As can be seen, this isn't helpful.
>
> Similar report: [#29398](https://github.com/bitcoin/bitcoin/issues/29398)
>
> [@Fuzion24](https://github.com/Fuzion24) It would be helpful if the issue could be reproduced step-by-step on a freshly created regtest testing wallet.

Thanks a lot @maflcko. I can that in good faith. I had an idea and used an LLM to try to make it more professional I guess that was my ba
...

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2945481917)
Rebased 5b38e62ccbfcdb7f20a6a74fcc4ef2358e3da56c -> c1cce04f763e88347cf025fab76b894b4b015dab ([`pr/ipc-stop.4`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.4) -> [`pr/ipc-stop.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.4-rebase..pr/ipc-stop.5)) due to conflicts with #32641 and #32438
Updated c1cce04f763e88347cf025fab76b894b4b015dab -> a180915d3604a2bff14040ccbfd67ee586a8e212 ([`pr/ipc-stop.5`](
...

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2129634568)
I went pretty far into implementing this, but realized there is a downside to this approach - the set stores entries for each announcement instead of each unique orphan. It requires us to update this map each time a new announcement is added/removed instead of just for unique ones. Perhaps worth keeping as `Wtxid` - what do you think?

💬 ryanofsky commented on pull request "ci: Rewrite test-each-commit as rust script":
(https://github.com/bitcoin/bitcoin/pull/32680#issuecomment-2945538207)
If I compare the [added rust script](https://github.com/bitcoin/bitcoin/blob/fa9aced60a320904405e4add6998fb20e64cd7e5/.github/ci-test-each-commit-exec.rs) to the [removed bash script](https://github.com/bitcoin/bitcoin/blob/a980918f51d710806acbb3bc4e77cb6be63cc04b/.github/ci-test-each-commit-exec.sh), the bash script does seem significantly simpler to me. I understand the rust script could have other improvements and maintenance advantages though, so no objections to this change.

💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2129683785)
Ah, I see. So you could have an $\mathcal{O(n)}$ blowup factor with $n$ the number of peers that have announced that Wtxid? If so, that doesn't sound like a worthwhile tradeoff.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129737657)
done - I agree. The removal from `setBlockIndexCandidates` will be (unnecessary) because at this stage the index cannot be in that set - `ReceivedBlockTransactiosns` is only called later.
But this is not on a performance-critical path (since behind PoW) so the added removal shouldn't matter.

>From my (limited) understanding, think state.IsInvalid() should always evaluate to true and thus seems like more of an assertion, but it's orthogonal to my suggestion.

I don't think calling `Invalid
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129738402)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129740992)
good point - yes, that comment belonged to the following commit. I moved it there in a slightly shorter form.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129741569)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129780775)
I think that this behavior is because the existing behavior during normal addition ([here](https://github.com/bitcoin/bitcoin/blob/ae024137bda9fe189f4e7ccf26dbaffd44cbbeb6/src/node/blockstorage.cpp#L230)), is to set it to the header received first, and it is
not changed in `ReceivedBlockTransactions` based on which full blocks are first received in full/connected.

So in this scenario
1.) Receive Header 1
2.) Receive Header 2 of same height
3.) Receive/Connect Block 2
4.) Receive Block 1
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2129781087)
fixed