Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 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)
👋 fanquake's pull request is ready for review: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(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
...
💬 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
💬 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.
💬 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
💬 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
...
🤔 furszy reviewed a pull request: "test: Remove redundant verack check"
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107711418)
utACK 0000276b31ce
👍 BrandonOdiwuor approved a pull request: "log: use error level for critical log messages"
(https://github.com/bitcoin/bitcoin/pull/30255#pullrequestreview-2107735380)
ACK 2222f2573c8de0a7f7171e6fc9e844ad448e1f65 - Replacing LogPrintf with Logerror to enhance critical error logging
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107726655)
Code review ACK 09ef322acc0a88a9e119f74923399598984c68f6. Just fuzz test error checking fix and updated comment since last review
💬 ryanofsky commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1633207579)
In commit "[[refactor]] Check CTxMemPool options in constructor" (09ef322acc0a88a9e119f74923399598984c68f6)

It's a shame this has to return a pointer now. It would have been nice to avoid the unreachable code with something like [scope_exit](https://en.cppreference.com/w/cpp/experimental/scope_exit), but I guess that is still experimental and hasn't been standardized.

```c++
bilingual_str error;
auto error_check{std::experimental::scope_exit([&] { Assert(error.empty()); })};

...
🤔 vasild reviewed a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2107713174)
I think this was merged prematurely.
💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633216366)
Now it will not overflow `buf` but it will lose the trailing `N - M` bytes from the peek.

To fix this, when assigning `random_bytes` earlier, it should only take/pop the first `len` bytes from `m_peek_data`.

I think all this may be unnecessary if the app has to work with peek only returning 1 byte, as discussed in https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623330207
💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633199919)
`std::vector<uint8_t>` can be used instead of `std::optional<std::vector<uint8_t>>` and if the vector is empty that is the same as the optional without a value.
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633223401)
> This was the case before this PR, right?

No, a peek read before this PR would return 1 byte and a normal read following that would also only return that one byte (with the exception of sometimes padding with a bunch of zeros). This is what causes the fuzz input to never be interpreted as one continuous sequence of data, which is why mutations that insert pieces of data (e.g. from a dictionary) are ineffective.

> Where can I see that use case? Is there some code that does not work without
...
👍 ryanofsky approved a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-2107775468)
Code review ACK f68cba29b3be0dec7877022b18a193a3b78c1099. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.
📝 glozow opened a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260)
Backports:
- #30151

And final changes for 26.2rc1:
- build changes
- manpages