Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 MarnixCroes commented on pull request "debugwindow: update session ID tooltip":
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1910736573)
> Concept ACK
>
> nit: @MarnixCroes, maybe you can add the screenshot for transport v1 and session ID is empty/ not shown, if possible, even this PR doesn't affect that logic.

done
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466747189)
old comment?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466751943)
at the introductory commit, I don't see how `m_ancestors_of_in_package_ancestors` is non-empty. unused?

I suspect replaced by `package_with_ancestors` usage?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466727264)
`has_mempool_sibling` since I'm thinking in reference to `ptx` and it's a binary result?
🤔 instagibbs reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1844316858)
`PreCheck`s loop looks so much cleaner, but there's some dead code and like before I'm not super clear which checks are *required* in `AcceptMultipleTransactions` vs anti-DoS type checks. For example, does `ApplyV3Rules` matter inside `AcceptMultipleTransactions`?

I'd like it to be crystal clear to not introduce regressions, and moving forward to cluster mempool we keep all this in mind for future simplification.
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466746854)
can assert that `package_with_ancestors.ancestor_counts.size() == workspaces.size()`?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754879)
unused
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466744432)
random newline
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754974)
unused
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1910755371)
concept ACK, for the record
📝 1amhesus opened a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
1amhesus closed a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
📝 fanquake locked a pull request: "[temp] Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1910846023)
I switched my own machine from Docker to Podman. This removes the need for 7cdb75d572e825b4c0cf74a516f912f94807853f so that's dropped.

Added a commit that explains the process a bit more clearly.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466829063)
These lines are dropped now.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466829719)
I nuked Docker in favor of Podman. So far it seems to work. I dropped 7cdb75d572e825b4c0cf74a516f912f94807853f.
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466864661)
Was thinking about why this is ok. AcceptMultipleTransactions doesn't allow RBF, and future package RBF should work with this?

```suggestion
// The mempool or in-package parent cannot have any other in-mempool children.
// We don't need to handle the "single RBF" case as that is
// handled in ApplyV3Rules for single transaction packages.
// Future package RBF would conflict with the parent as well,
// making this check sufficien
...
🤔 mzumsande reviewed a pull request: "test/BIP324: functional tests for v2 P2P encryption"
(https://github.com/bitcoin/bitcoin/pull/24748#pullrequestreview-1844567036)
Code Review ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3
💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1910905907)
@ajtowns, to try to steer this discussion in a more constructive direction, would it be possible for you to make a list of the biggest harms you believe this PR would cause if it were merged? And if you have ideas on how the harms you see could be avoided, while the problems I am trying to solve (see "Problems this PR attempts to solve" in the PR description) could be addressed, it would be really helpful to know about. Maybe the harms you see this PR causing and the problems this PR is trying t
...
💬 achow101 commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1910911055)
ACK d298ff8b62b2624ed390c8a2f905c888ffc956ff