Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574616580)
done
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574633307)
deleted
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574616703)
deleted
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1574616498)
deleted
👍 fjahr approved a pull request: "doc: explain what the wallet password does"
(https://github.com/bitcoin/bitcoin/pull/28974#pullrequestreview-2014630337)
ACK d07557d0038521f78baf3f1c94e1f282f51b8c1b

Could still be condensed a bit but this is alright.
💬 fjahr commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1574679210)
nit: This is extremely wordy, could be shortened to one sentence since this is just an intro.
💬 fjahr commented on pull request "doc: explain what the wallet password does":
(https://github.com/bitcoin/bitcoin/pull/28974#discussion_r1574678252)
nit: I'm not sure if we need to give general IT security advice. I would probably remove that part.
👍 theStack approved a pull request: "test: Fix intermittent issue in p2p_handshake.py"
(https://github.com/bitcoin/bitcoin/pull/29898#pullrequestreview-2014656748)
Tested ACK 6b02c11d667adff24daf611f9b14815d27963674
📝 maflcko opened a pull request: "test: Fix intermittent timeout in p2p_tx_download.py"
(https://github.com/bitcoin/bitcoin/pull/29933)
Currently the test passes, but may fail during shutdown, because blocks are synced with `NUM_INBOUND` * `self.num_nodes` peers, which may take a long time.

There is no need for this test to have this amount of inbounds.

So avoid the extraneous inbounds to speed up the test and avoid the intermittent test failures.
👋 paplorinc's pull request is ready for review: "refactor: Reduce memory copying operations in bech32 encoding/decoding"
(https://github.com/bitcoin/bitcoin/pull/29607)
💬 fjahr commented on pull request "test: Fix multiprocess CI timeout in p2p_tx_download":
(https://github.com/bitcoin/bitcoin/pull/29926#discussion_r1574720880)
I initially had has only used the `node.setmocktime(0)` in that place and that succeeded when I temporarily pushed it in an unrelated PR but then here [it failed when I opened this one](https://github.com/bitcoin/bitcoin/runs/24062801267). After adding the +300 it succeeded, but yeah, this may have been just luck again. I will push a version without +300 and and the `node.setmocktime(0)` moved higher up.

I just saw that the Re-Run button also appears for CI jobs that succeeded, I will try to
...
💬 maflcko commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1574722297)
Putting this here will spin up `NUM_INBOUND` * `self.num_nodes`, which seems confusing, because they are not needed.

Also, it will cause the test thread to sync all generated blocks, blocking shutdown, and causing intermittent test failures.

Fixed in https://github.com/bitcoin/bitcoin/pull/29933
💬 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?