💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1446216031)
It's been a while since i wrote this code and to be honest i don't remember what i was thinking. (Note-to-self: comments are good actually.)
As you point out it does change the behaviour of the existing target. Since i don't have a rationale i've updated it to simply keep the existing behaviour.
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1446216031)
It's been a while since i wrote this code and to be honest i don't remember what i was thinking. (Note-to-self: comments are good actually.)
As you point out it does change the behaviour of the existing target. Since i don't have a rationale i've updated it to simply keep the existing behaviour.
💬 jonatack commented on pull request "[26.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29011#issuecomment-1883240726)
Suggest backporting #29200 that resolves https://github.com/bitcoin/bitcoin/issues/29197.
(https://github.com/bitcoin/bitcoin/pull/29011#issuecomment-1883240726)
Suggest backporting #29200 that resolves https://github.com/bitcoin/bitcoin/issues/29197.
💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1883240915)
> Spend.cpp coverage is not too bad already: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/spend.cpp.gcov.html
>
> It may be good to clarify the new coverage a bit.
I didn't understand. Are you suggesting that I reconsider what I want to increase coverage of in this file?
Also, I have a question about the blue lines in the coverage reports. Do they represent the line coverage of the code during fuzzing? For instance, if these functions are encountered in the process of runn
...
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1883240915)
> Spend.cpp coverage is not too bad already: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/spend.cpp.gcov.html
>
> It may be good to clarify the new coverage a bit.
I didn't understand. Are you suggesting that I reconsider what I want to increase coverage of in this file?
Also, I have a question about the blue lines in the coverage reports. Do they represent the line coverage of the code during fuzzing? For instance, if these functions are encountered in the process of runn
...
💬 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
...
(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
(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.
(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.
(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.
(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
(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)
(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
(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
(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
(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.
(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)
(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.
(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.
(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
(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.
(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.
(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.