Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 brunoerg approved a pull request: "fuzz: Fix difficulty target generation in `p2p_headers_presync`"
(https://github.com/bitcoin/bitcoin/pull/31213#pullrequestreview-2448363822)
code review ACK a6ca8f324396522e9748c9a7bbefb3bf1c74a436
📝 Abdulhafidh-TripleA opened a pull request: "Update README triple A.md"
(https://github.com/bitcoin/bitcoin/pull/31330)
bitcoin to triple A

<!--
*** 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 o
...
willcl-ark closed a pull request: "Update README triple A.md"
(https://github.com/bitcoin/bitcoin/pull/31330)
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31330)
bitcoin to triple A

<!--
*** 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 o
...
🚀 fanquake merged a pull request: "fuzz: Fix difficulty target generation in `p2p_headers_presync`"
(https://github.com/bitcoin/bitcoin/pull/31213)
📝 fanquake opened a pull request: "doc: add copyright header to p2p_headers_presync"
(https://github.com/bitcoin/bitcoin/pull/31331)
Add the missing copyright header.
👍 willcl-ark approved a pull request: "doc: add copyright header to p2p_headers_presync"
(https://github.com/bitcoin/bitcoin/pull/31331#pullrequestreview-2448419190)
ACK 7d3703dec3d738204257b4eb77e4e83d1c79e23c
🚀 fanquake merged a pull request: "doc: add copyright header to p2p_headers_presync"
(https://github.com/bitcoin/bitcoin/pull/31331)
🤔 glozow reviewed a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122#pullrequestreview-2448349325)
ACK 5736d1ddacc

I really like how `ChangeSet` codifies the concept of an atomic changeset of additions and removals, and how `CalculateChunksForRBF` and `ImprovesFeerateDiagram` operate on changesets. It's like the mempool interface is moving towards it being more responsible for itself.
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850234021)
nit: these changes were perhaps meant for d7dc9fd2f7bc675256687b9c55fdbec9cc8ac781 instead of 284a1d33f1dcbc3b3404ea40a948ff6600239613?
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850146937)
nitty, but I wondered why not `CTransactionRef` here?

```suggestion
CTransactionRef GetAddedTxn(size_t index) const { return m_entry_vec.at(index)->GetSharedTx(); }
```
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850173724)
I misunderstood this comment initially, thinking it meant that you would call both `addNewTransaction`s, but it actually means that one is a wrapper for the other, with a prepended CMPA. Callers should use one or the other.
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850204863)
b447416fddcb8c8647391502cca3dbfd1552e02e is nice!
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850244902)
Sorry about this comment - just reread what I said and it makes no sense.
🤔 BrandonOdiwuor reviewed a pull request: "RFC: build: support for pre-compiled headers."
(https://github.com/bitcoin/bitcoin/pull/31053#pullrequestreview-2448510169)
Concept ACK using pre-compiled headers to speed up compilation
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1850252005)
for reference, there are new CI machines lined up, but they can't be deployed due to https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2459823076
🚀 glozow merged a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122)
💬 dergoegge commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2488561557)
Perhaps this makes more sense in combination with a test that uses it? Currently `FuzzedSock::Accept` seems entirely unused: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/test/fuzz/util/net.cpp.gcov.html
💬 kevkevinpal commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850347673)
maybe we can update the error message to be
```
ExitFailure("Cannot obtain a lock on test data lock directory " + fs::PathToString(m_path_lock) + '\n' + "The directory and .lock file already exist, please ensure the directory .lock file does not exist.");
```

it is fine if the directory exists, we are just don't want `.lock` file to be present?
I can see if I can come up with better wording than what I suggested above
💬 kevkevinpal commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850374636)
updated in fe6da3744420bcbac9002ae324dfa04902aed490