💬 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.

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

Thanks, in this case it seems the issue is transactions relayed to the extraneous peers. I've edited the pull request description.

To see what message is the cause for the block, you can inspect the full combined log. In this example:


```
...
[0;36m test 2024-04-19T18:08:46.732000Z TestFramework.p2p (DEBUG): Received message from 127.0.0.1:17361: msg_tx(tx=CTrans
...