💬 fanquake commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3468059025)
https://github.com/bitcoin/bitcoin/actions/runs/18939943748/job/54075967306?pr=33745#step:9:2351:
```bash

test/miner_tests.cpp(70): Entering test suite "miner_tests"
test/miner_tests.cpp(686): Entering test case "CreateNewBlock_validity"
<snip>
2025-10-30T12:08:21.221330Z [test] [validationinterface.cpp:218] [void ValidationSignals::BlockConnected(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *)] [validation] Enqueuing BlockConnected: block hash=000000000019d66
...

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3468078851)
> What is the plan for adding CI, as that blocks everything here?

I'm working on it. I have issues with the `sqlite` package that are not reproducible neither in the [nightly builds](https://github.com/hebasto/bitcoin-core-nightly/actions/workflows/windows-gcc-ucrt.yml?query=event%3Aschedule) nor locally.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2478177222)
I move the change away from here.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3468101541)
@TheCharlatan wrote:

> What about the other checked flags? Should we just reset all of them in `AddMerkleRootAndCoinbase`?

Done and pushed, but still checking the CI failure.

💬 fanquake commented on pull request "doc: add coverage instrumentation hint to libFuzzer quickstart":
(https://github.com/bitcoin/bitcoin/pull/33536#issuecomment-3468116092)
@frankomosh the PR description needs updating to reflect the current change?

cc @dergoegge @marcofleon

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3468117727)
I dropped `Assume(pblock->m_checked_witness_commitment);` from `miner.cpp`, since the assumptions seems to be wrong.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2478202068)
It behaves the same way as `ProcessNewBlock`, which has the following confusing documentation:

> This only returns after the best known valid
> * block is made active. Note that it does not, however, guarantee that the
> * specific block passed to it has been checked for validity!

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2478209632)
I do think it would be nice to return clear errors somehow, but that seems a separate issue.

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2478210721)
Done

💬 fanquake commented on issue "RFC: Add multiprocess fuzz target":
(https://github.com/bitcoin/bitcoin/issues/23015#issuecomment-3468145119)
Note that capnp has it's own detection for if it's being fuzzed, which was added as part of its oss-fuzz integration (https://github.com/capnproto/capnproto/pull/1188). This will likely need to be patched out/worked around at some point. See https://github.com/google/oss-fuzz/pull/14203.

👍 TheCharlatan approved a pull request: "test: Add bitcoin-chainstate test for assumeutxo functionality"
(https://github.com/bitcoin/bitcoin/pull/33728#pullrequestreview-3399808856)
ACK 3e7d272b2747f1937010b8fc1fefafcfa0ce3213

💬 maflcko commented on pull request "fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn":
(https://github.com/bitcoin/bitcoin/pull/33743#issuecomment-3468260631)
Yeah, I guess it is too early to call this a bug, but the language standard has been changed, see https://github.com/bitcoin/bitcoin/pull/33644#issuecomment-3430890579.

I've used your wording for now.

💬 maflcko commented on issue "valgrind: Conditional jump or move depends on uninitialised value(s)":
(https://github.com/bitcoin/bitcoin/issues/29635#issuecomment-3468267477)
I think this is hopeless, even with `-O1`, using the following diff, it fails.

```diff
sh-5.2# git log -1 --oneline
72511fd (grafted, HEAD -> master, origin/master, origin/HEAD) Merge bitcoin/bitcoin#33555: build: Bump clang minimum supported version to 17
sh-5.2# git diff
diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh
index d6c4575..b64648f 100755
--- a/ci/test/00_setup_env_native_valgrind.sh
+++ b/ci/test/00_setup_env_native_valgrind.sh
@@ -8,7
...

✅ maflcko closed a pull request: "doc: Document valgrind aarch64 clang workaround"
(https://github.com/bitcoin/bitcoin/pull/33742)

💬 maflcko commented on pull request "doc: Document valgrind aarch64 clang workaround":
(https://github.com/bitcoin/bitcoin/pull/33742#issuecomment-3468281402)
Closing for now

💬 GregTonoski commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-3468291452)
> For the record, this pull-req wasn't my idea. I was asked to open it by an active Core dev because entities like Citrea are using unprunable outputs instead of OP_Return, due to the size limits. - @petertodd, https://stacker.news/items/971277?commentId=971434

💬 Sjors commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2478319678)
Thinking about adding the following to this comment:

```cpp
/** Update uncommitted block structures (currently: only the witness reserved
* value). This is safe for submitted blocks as long as they honor
* default_witness_commitment from the template. */
void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev) const;
```

💬 instagibbs commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3468315631)
@Crypt-iQ `BlockEncoding.*` benchmarks should cover this up to 50k. Quickly tested 100k, and it seems ~linear slowdown.

💬 yuvicc commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#issuecomment-3468350669)
Concept ACK

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3468367525)
> the second peer can endlessly send CMPCTBLOCK's

If we're concerned about that, seems better to fix it directly?

```diff
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4412,6 +4412,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
while (range_flight.first != range_flight.second) {
if (range_flight.first->second.first == pfrom.GetId()) {
requested_block_from_this_peer = true;
+
...