Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 moonsettler commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883258307)
> From the [contributing guidelines](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review)
>
> > **After** conceptual agreement on the change, code review can be provided.
>
> I can only speak for myself obviously but Concept NACK from me for this repo at the current time. This pull request should be opened to bitcoin-inquisition or a fork of Core with its own custom signet consensus rules.

You are clearly commenting in bad faith. But in the off chance you want to
...
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883272800)
@theuni See https://github.com/bitcoin/bitcoin/pull/29208
💬 michaelfolkson commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883279066)
@moonsettler: That PR is in draft, was opened to bitcoin-inquisition simultaneously and doesn't seem to have conceptual agreement at the current time either. Everyone opening their own pull request to this repo with their own favorite combination of opcodes and sighash flags wastes people's time. They will have to come to this pull request and tell you exactly what Sjors and I have already told you. But if you insist that is what they will have to do.
👍 hebasto approved a pull request: "build: Bump clang minimum supported version to 14"
(https://github.com/bitcoin/bitcoin/pull/29208#pullrequestreview-1811425126)
ACK fa223ba5eb764fe822229a58d4d44d3ea83d0793.
💬 hebasto commented on pull request "build: Bump clang minimum supported version to 14":
(https://github.com/bitcoin/bitcoin/pull/29208#discussion_r1446254597)
nit: I understand that that was a clean commit revert. But these lines look really unneeded these days.
💬 glozow commented on pull request "[26.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29011#issuecomment-1883287324)
I'm planning to merge this now and open another batch of backports
🚀 glozow merged a pull request: "[26.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29011)
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 14":
(https://github.com/bitcoin/bitcoin/pull/29208#discussion_r1446267614)
Thanks, fixed
💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1446269441)
changed
💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1446269555)
taken
💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883309957)
Hi @michaelfolkson, can you enumerate a concrete dispute with my proposals (on their respective PRs) or my code here? Otherwise, I'm not sure your comments are contributing to the discussion. You've registered your Concept NACK.
🚀 fanquake merged a pull request: "build: Drop `ALLOW_HOST_PACKAGES` support in depends"
(https://github.com/bitcoin/bitcoin/pull/29203)
💬 michaelfolkson commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883319863)
@reardencode: No, registering my Concept NACK is sufficient. I'll leave it to others to rehash what Sjors and I have already advised and you can choose to ignore them too.
💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446286078)
You need it for the logs iirc.
📝 glozow opened a pull request: "[26.x] more backports"
(https://github.com/bitcoin/bitcoin/pull/29209)
Backports for 26.x. Includes:
- 453b481 from #28391
💬 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?