💬 fjahr commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069361083)
@maflcko did you see #29926?

💬 maflcko commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069380043)
> @maflcko did you see #29926?

It looks like 29926 is adding a call to `setmocktime` in the test. As the issue happens *after* all tests have passed, this can not fix the issue. Or am I missing something?

To explain it a bit more, the traceback is:

```
File "/test/functional/p2p_tx_download.py", line 302, in <module>
TxDownloadTest().main()
File "/test/functional/test_framework/test_framework.py", line 155, in main
exit_code = self.shutdown()
^^^^^^^^^^^
...

💬 fjahr commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069404140)
Hm, do you have an example failure where you have seen that traceback? Or is this from reproducing it locally? Because I haven't seen this, at least not the shutdown part. Here is one of the ones I have seen and it seems to be a different failure: https://cirrus-ci.com/task/5622109341220864

💬 MartiPresa commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-2069433861)
Is this issue still available?

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574737173)
At first when I see CPFP carveout disabled I thought we are not longer supporting single transaction CPFP carveout because the linked document is describing rules for a single transaction CPFP carveout.

But from the commit message and reading the code I understand thats not the case, I think we should be explicit that it's package CPFP carvout that is disabled.

```suggestion
* [Package CPFP Carve Out](./mempool-limits.md#CPFP-Carve-Out) is disabled. (#21800)
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574744045)
Maybe test that Package RBF carveout is not supported?
```suggestion

# But We can not package RBF the chain which used our carveout rule
replaceable_tx_conflict = self.wallet.create_self_transfer_multi(utxos_to_spend=[chain[0]])
txns_conflict = [replaceable_tx_conflict["tx"], self.wallet.create_self_transfer_multi(utxos_to_spend=replaceable_tx_conflict["new_utxos"])["tx"]]
txns_conflict_hex = [tx.serialize().hex() for tx in txns_conflict]
assert_eq
...

🤔 ismaelsadeeq reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2014725944)
Continued Reviewing.

- [x] 826ec4c47c0d18a2d9b437b33ea4ceba67e870c1
- [x] 033736bcd9fc16e244e52e72fe7c7ff030690ece

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574739231)
This also implies that [single conflict package RBF carvout](https://github.com/bitcoin/bitcoin/pull/23711#discussion_r765766768) is not supported, is this worth mentioning?

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574741503)
```suggestion
# Ensure an individual transaction with single direct conflict can RBF the chain which used our carve-out rule

```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1574766371)
In `packages.md` there is note stating "Replace By Fee is currently disabled for packages." This is no longer the case, some variants of package can now be RBF'd I think we should indicate that we now support cluster size 2 package replacement into node's mempool stating the new acceptance rules.

💬 maflcko commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069448527)
To get the full error log on Cirrus CI, you'll have to use the "Open Full Logs" button to see the full log. Usually the output is:

... (dots from the test runner)
traceback (from the failed test)
Full combined logs (from the failed test)
Full test summary (from the test runner)

💬 sipa commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#issuecomment-2069471388)
Rebased after the merge of #29879.

💬 jrakibi commented on pull request "test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply":
(https://github.com/bitcoin/bitcoin/pull/29617#discussion_r1574776450)
Done 👍

💬 glozow commented on pull request "test: Fix intermittent timeout in p2p_tx_download.py":
(https://github.com/bitcoin/bitcoin/pull/29933#issuecomment-2069499720)
I can see the `self.shutdown()` traceback in this one: https://api.cirrus-ci.com/v1/task/6740372997537792/logs/ci.log

💬 jrakibi commented on pull request "test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply":
(https://github.com/bitcoin/bitcoin/pull/29617#issuecomment-2069508021)
@fjahr I've updated the code based on your feedback. In the commit description, you can find the steps followed to arrive at the content values, along with a tool to decode a UTXO snapshot

✅ maflcko closed an issue: "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. "
(https://github.com/bitcoin/bitcoin/issues/29789)

💬 instagibbs commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2069528455)
> Seems like this goes beyond the expected behaviour. Maybe make it =2 at least?

Since these are test-network-only changes, are we required to preserve current behavior?

💬 theuni commented on pull request "[WIP] libevent @ master + use CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2069538101)
I believe (@fanquake can confirm) that the idea was to test the full merge, and if we liked the CMake build parts we'd patch them in and drop the rest.

💬 Sjors commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2069559533)
My remark is orthogonal to #29918. macOS < 13 doesn't have clang 15. They also don't have clang 14, which was missed in #29208.

There's need to make a separate pull request for a single sentence:

At the bottom of step 3:

```md
On macOS < 13 you additionally need to install a more recent version of clang:

brew install clang
```

💬 fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#discussion_r1574814671)
Had another failure, so I will add the +300 back and see if that is a permanent fix.