Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1883380383)
pushed update with fee diagram checks instead of heuristics. @sipa please see https://github.com/bitcoin/bitcoin/pull/28984/commits/9dda95d58442e4884a57216472c991c62f87ef1f and similar

Addressing other comments next.
🤔 stratospher reviewed a pull request: "p2p: attempt to fill full outbound connection slots with peers that support tx relay"
(https://github.com/bitcoin/bitcoin/pull/28538#pullrequestreview-1811535657)
ACK 8f07458. useful to fill outbound slots with peers which we can relay transactions with and i liked the approach in this PR to swap away blocks-only peers in full relay outbound slots only after the threshold number of max full relay outbound connections is reached.
💬 stratospher commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1446322434)
68769ff: nit if you retouch.
```suggestion
with self.nodes[0].assert_debug_log(expected_msgs=["disconnecting full outbound peer not participating in tx relay: peer=1"]):
self.nodes[0].mockscheduler(46)
blocksonly_peer2.wait_for_disconnect()
```
💬 stratospher commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#discussion_r1446331630)
> It would be easy to exempt non-tx-relaying m_protect peers from disconnection, but that would mean that we could end up with up to 4 of these peers permanently.

[commit message](https://github.com/bitcoin/bitcoin/pull/11490/commits/5a6d00c6defc587e22c93e63029fdd538ce8858d) where `m_protect` was introduced says "we pick 4 of our outbound peers and do not subject them to this logic, to be more conservative. We don't wish to permit temporary network issues (or an attacker) to excessively disru
...
💬 maflcko commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1446339387)
nit: Forgot to adjust the cirrus yaml description of this task?
💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm":
(https://github.com/bitcoin/bitcoin/pull/29188#issuecomment-1883429315)
I'm just going to squash this back into #21778, and get it done together. The changes (combined with lld) are now working.
fanquake closed a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG` & native_llvm"
(https://github.com/bitcoin/bitcoin/pull/29188)
fanquake closed an issue: "I2P: Change encryption type"
(https://github.com/bitcoin/bitcoin/issues/29197)
🚀 fanquake merged a pull request: "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption"
(https://github.com/bitcoin/bitcoin/pull/29200)
💬 achow101 commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1883456495)
ACK fb5bfed26a564014b83ccfc96ff00b630930fc61
💬 mzumsande commented on pull request "p2p: attempt to fill full outbound connection slots with peers that support tx relay":
(https://github.com/bitcoin/bitcoin/pull/28538#issuecomment-1883481846)
Thanks for the review!

We had an offline conversation about this today (FYI @sdaftuar @sipa @sr-gi). Some takeaways:
- It would probaly be better to first have a replacement peer ready before disconnecting a blocksonly peer. That would mean a similar approach as for the extra network-specific peers, it would also mean that we would need to add a way to make extra connections when we have a -blocksonly peer we want to get rid of.
- We should avoid situations where there could be perpetual ch
...
📝 mzumsande converted_to_draft a pull request: "p2p: attempt to fill full outbound connection slots with peers that support tx relay"
(https://github.com/bitcoin/bitcoin/pull/28538)
As described in the issues #16418 and #28371, it's a possibility that we could end up having an insufficient number of outbound peers relaying our transactions. Having fewer outbound peers support tx-relay could also have implications on [privacy](https://github.com/bitcoin/bitcoin/issues/16418#issuecomment-514328042) and fee estimation.

While #28488 is suggesting meaures based on comparing fee filters / mempool sizes, there is also the simpler issue of peers that tell us they don't want tran
...
💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446405052)
> Since all of the different ways that sffo is set by the user, this could do type interpretation to figure it out.

That makes sense, what do you think about doing the type interpretation on the UniValue passed in, instead of relying on the key names? I'd rather do that than try to have this function take a `std::variant` or something like that
🚀 achow101 merged a pull request: "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo"
(https://github.com/bitcoin/bitcoin/pull/29058)
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434642)
Pretty sure following section covers it, adding two descendants one by one to `tx10` and checking the resulting failure
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434675)
taken
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434720)
cleaner, thanks. will take(minus typos :) )
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434808)
this all got moved internal to mempool
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434903)
replaced entirely(I think?)
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446434933)
replaced section of code entirely