🤔 glozow reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2107324391)
looks pretty good 7b74cbf6ed7cafdbd458471ae89fa88c52256278

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633036250)
Needs rebase for #29496?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1632980812)
nit: 7d895935aef63b4aa728310b580f418ce35fc234

we started using `low_fee_amt` instead of 200 in this test

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633026172)
Can check `m_wtxids_fee_calculations` and `m_effective_feerate`.

Wrote up the changes for this + using `CheckPackageMempoolAcceptResult`:
```diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index cd69fce2f5..b68686cfcb 100644
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -975,28 +975,22 @@ BOOST_FIXTURE_TEST_CASE(package_rbf_tests, TestChain100Setup)

LOCK(m_node.mempool->cs);
const auto submit1 = ProcessNewPackage(
...

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633053605)
No mentions of BIP125 outside of signaling pls
```suggestion
- Replacements must pay more total total fees at the incremental relay fee (analogous to regular [replacement rules](./mempool-replacements.md) 3 and 4).
```

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633039485)
aa3a4da248fe9d4a765fa4c80797ca333ee37f07: is the static cast necessary?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1632990659)
Should use `CheckPackageMempoolAcceptResult` to check that results are populated as expected, which also means you can remove a few of these checks:

```suggestion
if (auto err_1{CheckPackageMempoolAcceptResult(package1, submit1, /*expect_valid=*/true, m_node.mempool.get())}) {
BOOST_ERROR(err_1.value());
}
auto it_parent_1 = submit1.m_tx_results.find(tx_parent->GetWitnessHash());
auto it_child_1 = submit1.m_tx_results.find(tx_child_1->GetWitnes
...

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633049147)
91a7e8ba7606b68401b29d2609c240a604ab6fe1: this isn't a perfect test for cluster size 2... why not call `CheckConflictTopology`?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1633045871)
5dd74f9c6ddc218417bda841f493ca2ed3f2b327

I don't 100% remember why we did this, but I think it was so we don't check mempool contents when `test_accept=true`?

🚀 fanquake merged a pull request: "build: Remove --enable-gprof"
(https://github.com/bitcoin/bitcoin/pull/30257)

💬 stickies-v commented on pull request "log: use error level for critical log messages":
(https://github.com/bitcoin/bitcoin/pull/30255#discussion_r1633060350)
Can't we just merge them?

<details>
<summary>git diff on b90f1c36ff</summary>

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 89aa84a551..ac95824443 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -6303,12 +6303,10 @@ bool ChainstateManager::ValidatedSnapshotCleanup()
fs::path p_old,
fs::path p_new,
const fs::filesystem_error& err) {
-
...

💬 fanquake commented on pull request "build: Remove --enable-gprof":
(https://github.com/bitcoin/bitcoin/pull/30257#issuecomment-2158040840)
Removed the label because nothing was ever ported.

💬 stickies-v commented on pull request "log: use error level for critical log messages":
(https://github.com/bitcoin/bitcoin/pull/30255#issuecomment-2158082194)
re-ACK 2222f2573c8de0a7f7171e6fc9e844ad448e1f65

💬 maflcko commented on pull request "build: Remove --enable-gprof":
(https://github.com/bitcoin/bitcoin/pull/30257#issuecomment-2158089097)
It needs to be ported to the table in https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2158091611)
The false positive CI error still happens. I am taking another look.

💬 hebasto commented on pull request "build: Remove --enable-gprof":
(https://github.com/bitcoin/bitcoin/pull/30257#issuecomment-2158131886)
> It needs to be ported to the table in https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e

Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1633124579)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#issuecomment-2158146838)
Made a few small changes still:

```diff
@@ -107,7 +107,11 @@ class IntBitSet
return *this;
}
/** Get the current bit position (only if != IteratorEnd). */
- constexpr const unsigned& operator*() const noexcept { return m_pos; }
+ constexpr unsigned operator*() const noexcept
+ {
+ Assume(m_val != 0);
+ return m_pos;
+ }
};

public:
@@ -231,6 +235,8 @@ class MultiIntBitSet
static constex
...

🤔 shaavan reviewed a pull request: "consensus: Store transaction nVersion as uint32_t"
(https://github.com/bitcoin/bitcoin/pull/29325#pullrequestreview-2107588105)
ACK 429ec1aaaaafab150f11e27fcf132a99b57c4fc7 💯

👋 fanquake's pull request is ready for review: "[27.1] Finalize"
(https://github.com/bitcoin/bitcoin/pull/30222)