💬 tdb3 commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1671493406)
nit: The comment below should also be adjusted. The case where the header is available but we don't have the block yet is covered by this new `JSONRPCError`.

// Block not found on disk. This could be because we have the block
// header in our index but not yet have the block or did not accept the
// block. Or if the block was pruned right after we released the lock above.

💬 tdb3 commented on pull request "rpc, rest: Improve getblock error when only header is available":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1671495550)
nit: Same for here

// Block not found on disk. This could be because we have the block
// header in our index but not yet have the block or did not accept the
// block. Or if the block was pruned right after we released the lock above.

💬 pythcoiner commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2219635092)
note: overlapping multipath are allowed
```python
self.test_importdesc({"desc": descsum_create(f"wsh(and_v(v:pk({xpriv}/<0;1>/*),or_d(pk({xpriv}/<1;2>/*),older(1000))))"),
"active": True,
"range": 10,
"timestamp": "now"},
success=True,
wallet=w_multipath)
```
but I don't see how it can be harmfull as it ends up in valid miniscript
...

💬 pythcoiner commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2219684402)
importing a descriptor w/ `"internal": false` fail because checks [here](https://github.com/bitcoin/bitcoin/blob/00337ef0e115f8bd8c1ede425f21ae7a1b6d30de/src/wallet/rpc/backup.cpp#L1073) and [here](https://github.com/bitcoin/bitcoin/blob/00337ef0e115f8bd8c1ede425f21ae7a1b6d30de/src/wallet/rpc/backup.cpp#L1476) are against if the key exist, not against its value.

```shell
~/cpp/bitcoin/src pr22838 *1 ?3 ❯ bitcoin-cli -regtest -rpcwallet=liana_recovery importdescriptors "[{\"desc\": \"wsh(or_d
...

👋 maflcko's pull request is ready for review: "util: Catch translation string errors at compile time"
(https://github.com/bitcoin/bitcoin/pull/30383)

💬 maflcko commented on pull request "random: add benchmarks and drop unnecessary Shuffle function":
(https://github.com/bitcoin/bitcoin/pull/30396#issuecomment-2219886162)
For fuzzing, just to keep it in mind, there is (obviously) no guarantee by the standard as to how `std::shuffle` behaves. See also https://en.cppreference.com/w/cpp/algorithm/random_shuffle#Notes

> Note that the implementation is not dictated by the standard, so even if you use exactly the same RandomFunc or URBG (Uniform Random Number Generator) you may get different results with different standard library implementations.

So going forward, I guess one can only assume to surely reproduce
...

⚠️ fanquake opened an issue: "ci: failure in `p2p_v2_misbehaving.py`"
(https://github.com/bitcoin/bitcoin/issues/30419)
https://github.com/bitcoin/bitcoin/actions/runs/9864841219/job/27240616054?pr=30410#step:7:5422
```bash
node0 2024-07-09T23:09:39.685000Z [net] [net.cpp:1814] [CreateNodeFromAcceptedSocket] [net] connection from 127.0.0.1:53666 accepted
test 2024-07-09T23:09:39.691000Z TestFramework (INFO): Sending remaining ellswift and garbage which are different from V1_PREFIX. Since a response is
test 2024-07-09T23:09:39.691000Z TestFramework (INFO): expected now, our custom data_received() funct
...

💬 fanquake commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#issuecomment-2219908962)
Looks like this is causing intermittent CI failures. See #30419.

💬 fanquake commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2219919953)
Seen recently here (`wallet_bumpfee.py --descriptors`) https://github.com/bitcoin/bitcoin/actions/runs/9863555844/job/27236587622#step:27:12496:
```bash
File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 188, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 1] Unable to connect to bitcoind after 2400s
test 2024-0
...

💬 maflcko commented on pull request "util: Catch translation string errors at compile time":
(https://github.com/bitcoin/bitcoin/pull/30383#issuecomment-2219940911)
Rebased to drop merged commit.

Current CI failure is unrelated and can be ignored.

💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#discussion_r1671897509)
I think it's a bit too verbose for the log but I added a comment. And I think it will be confusing to explicitly say everywhere in the normal case that the tip is an ancestor of the snapshot. But it's documented in one place now at least.

💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#discussion_r1671897648)
I think it's fine to keep the function name as is. The comment (now a log) provides the necessary context and I think it's just a special edge case of the less work behavior.

💬 fjahr commented on pull request "test, assumeutxo: Remove resolved todo comments and add new test":
(https://github.com/bitcoin/bitcoin/pull/30403#discussion_r1671897825)
Done

💬 fjahr commented on pull request "chainparams: Change nChainTx type to uint64_t":
(https://github.com/bitcoin/bitcoin/pull/29656#issuecomment-2219987968)
Rebased (caused by #30329)

💬 stratospher commented on issue "ci: failure in `p2p_v2_misbehaving.py`":
(https://github.com/bitcoin/bitcoin/issues/30419#issuecomment-2220039790)
oh a race condition happened because `initiate_v2_handshake()` code in `p2p_v2_misbehaving.py` was simplified. i think a simple `wait_until()` should fix it. will open a PR.

🤔 fjahr reviewed a pull request: "assumeUTXO: snapshot load, update VERSION msg height"
(https://github.com/bitcoin/bitcoin/pull/30418#pullrequestreview-2168560487)
I am still undecided if we want this and need to think about it a little more.

We send this usually when we have a new block but the loading of the snapshot does not necessarily mean that we have the block, right? We only need the header for that. So we should probably set it when we get block itself I think.

💬 fjahr commented on pull request "assumeUTXO: snapshot load, update VERSION msg height":
(https://github.com/bitcoin/bitcoin/pull/30418#discussion_r1671946079)
Are you sure this is the best place for this in the RPC? Looking at the other calls to `SetBestBlock()` they all seem to be happening in `validation.cpp`.

💬 fjahr commented on pull request "assumeUTXO: snapshot load, update VERSION msg height":
(https://github.com/bitcoin/bitcoin/pull/30418#discussion_r1671948758)
nit: You can also get the height out of `metadata` without the mutex (see below).

📝 stratospher opened a pull request: "test: Fix intermittent failure in p2p_v2_misbehaving.py"
(https://github.com/bitcoin/bitcoin/pull/30420)
Fixes #30419.

Make sure that ellswift computation is complete in the `NetworkThread` in `test/functional/p2p_v2_misbehaving.py` before sending the ellswift in the `MainThread`.


One way to reproduce this failure on master would be:

```diff
diff --git a/test/functional/test_framework/v2_p2p.py b/test/functional/test_framework/v2_p2p.py
index 87600c36de..ea0615ef3b 100644
--- a/test/functional/test_framework/v2_p2p.py
+++ b/test/functional/test_framework/v2_p2p.py
@@ -111,6 +111,7 @
...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2220126461)
> I don't think that benchmark is related, could just be noise.

Indeed, after your rebase the speed seems to be the same.