💬 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`?
(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`?
(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)
(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) {
-
...
(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.
(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
(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
(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.
(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.
(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.
(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
...
(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 💯
(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)
(https://github.com/bitcoin/bitcoin/pull/30222)
👋 fanquake's pull request is ready for review: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201)
(https://github.com/bitcoin/bitcoin/pull/30201)
💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1633146879)
I guess I am confused with the terms "active" and "background" chain. I assume the active chain is the snapshot chain which starts at height `299`, and the divergent chain is the one stuck at `298`. I base this on the fact that when I run `getbestblockhash` after `loadtxoutset`, I get the hash of the snapshot tip. However, I might be mistaken. Shouldn't the divergent chain be rewound to `START_HEIGHT` and become the background validation chain?
To address your questions:
1. The divergent c
...
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1633146879)
I guess I am confused with the terms "active" and "background" chain. I assume the active chain is the snapshot chain which starts at height `299`, and the divergent chain is the one stuck at `298`. I base this on the fact that when I run `getbestblockhash` after `loadtxoutset`, I get the hash of the snapshot tip. However, I might be mistaken. Shouldn't the divergent chain be rewound to `START_HEIGHT` and become the background validation chain?
To address your questions:
1. The divergent c
...
💬 brunoerg commented on issue "fuzz: crypter: Abrt in __cxxabiv1::failed_throw":
(https://github.com/bitcoin/bitcoin/issues/30251#issuecomment-2158194986)
Maybe it's not good to use `ConsumeRandomLengthByteVector`, worthes limiting its size.
https://github.com/bitcoin/bitcoin/blob/cad127235e307d7de0e9cc04708dbd31aa6c24b0/src/wallet/test/fuzz/crypter.cpp#L59
(https://github.com/bitcoin/bitcoin/issues/30251#issuecomment-2158194986)
Maybe it's not good to use `ConsumeRandomLengthByteVector`, worthes limiting its size.
https://github.com/bitcoin/bitcoin/blob/cad127235e307d7de0e9cc04708dbd31aa6c24b0/src/wallet/test/fuzz/crypter.cpp#L59
💬 fanquake commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#issuecomment-2158204184)
Added a commit to rename some references to macho ld64 to lld.
(https://github.com/bitcoin/bitcoin/pull/30201#issuecomment-2158204184)
Added a commit to rename some references to macho ld64 to lld.
💬 brunoerg commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2158209801)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30246#issuecomment-2158209801)
Concept ACK
💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633197400)
> We could make it so that a peek read only returns one byte and a subsequent normal read return N bytes where the first byte corresponds to the peek byte.
This was the case before this PR, right?
> But I'm not sure if that would change much for our use case.
Where can I see that use case? Is there some code that does not work without the changes to `FuzzedSock::Recv()` done in this PR?
The doc you cited, in my understanding, means that what I described in the first comment of this t
...
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633197400)
> We could make it so that a peek read only returns one byte and a subsequent normal read return N bytes where the first byte corresponds to the peek byte.
This was the case before this PR, right?
> But I'm not sure if that would change much for our use case.
Where can I see that use case? Is there some code that does not work without the changes to `FuzzedSock::Recv()` done in this PR?
The doc you cited, in my understanding, means that what I described in the first comment of this t
...
🤔 furszy reviewed a pull request: "test: Remove redundant verack check"
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107711418)
utACK 0000276b31ce
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107711418)
utACK 0000276b31ce