💬 hodlinator commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2570018072)
Hm.. I guess a shorter version of "test a limited number of ancestor commits" could work.
"test a few ancestor commits"?

💬 l0rinc commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2570025965)
It should signal that it's not exhaustive, something slightly ambiguous like "test latest commits" or "test latest few commits"?

💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3587403889)
> Please note that contributors are required to fully understand their authored code themselves.


The core change applies sigop limits to legacy scripts and fixes CLEANSTACK to only apply to P2SH as specified in BIP62. Most of the diff is test updates—the functional tests were using 0-sigop scripts that fail under the new policy, so they needed to be switched to 1-sigop scripts with proper signing.

I've been stuck on a `p2p_segwit.py` failure for quite a while now. The issue appears to be
...

💬 151henry151 commented on issue "node0 stderr bitcoind: validation.cpp:5392: void ChainstateManager::CheckBlockIndex() const: Assertion `!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))' failed.":
(https://github.com/bitcoin/bitcoin/issues/33948#issuecomment-3587488953)
This looks similar to #32173 (same line in CheckBlockIndex). Could the experimental LLVM 22 compiler be exposing a logic bug in setBlockIndexCandidates maintenance, similar to issues fixed in #16849 and #30479? The compiler might be hitting a consistency issue with setBlockIndexCandidates that happens when cs_main is released during block processing.

💬 fjahr commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#issuecomment-3587495871)
Had another thought: since there are quite a bit of small-ish helper functions refactored or replaced with different ones, it might be interesting to check a coverage report if there are any of these new functions that aren't covered and that coverage didn't go down somewhere in general. I didn't have time to look into this myself yet though.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2570548607)
They are sorted now and not sorted in the snippet you pasted above, e.g.:

```
ADDRMAN_ADDRESSES = [
"5.2.154.6",
"1.65.195.98",
"18.27.79.17",
"2.59.236.56",
```
I must be missing something...

💬 maflcko commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#issuecomment-3588184960)
lgtm ACK b5a7a685bba312a780eddcb4a53ce2c26a937854

👍 rkrux approved a pull request: "ci: Make the max number of commits tested explicit"
(https://github.com/bitcoin/bitcoin/pull/33909#pullrequestreview-3517851221)
crACK b5a7a685bba312a780eddcb4a53ce2c26a937854

💬 hodlinator commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2570754863)
"test latest commits" is incrementally better than master. But it is a bit ambiguous - especially for a newcomer who didn't know it by its old name, almost like if it were to be testing on master without any commits from the PR. Curious what others prefer.

👋 fanquake's pull request is ready for review: "[30.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33609)

💬 janb84 commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2570836209)
"test latest commits" would indicate to me that it would test all the commits in the PR (even if >6) , "test latest few commits" would be better imho.

Or even more contrarian, lean into curiosity and make it "test some latest commits"

👍 rkrux approved a pull request: "log: Use more severe log level (warn/err) where appropriate"
(https://github.com/bitcoin/bitcoin/pull/33960#pullrequestreview-3517968266)
crACK fa45a1503eee603059166071857215ea9bd7242a

The removal of the trailing `\n` seems fine as well because it is added later internally if not added by the caller.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2570863123)
Did a tweak commit on top of https://github.com/romanz/bitcoin/commit/9716ff4dd92b31fa4d97b909892f7fc8e3c82b9c: https://github.com/hodlinator/bitcoin/commit/0c75c322a404708d82d9bbf31b70f3b036c831ba

* NotFound -> IO seems more accurate.
* Add comments to explain why we don't log for some errors.
* Marginally nicer errors in rest.cpp (used contrived `0` for unset `part_size`). Feel less strongly about this.

🤔 janb84 reviewed a pull request: "ci: Make the max number of commits tested explicit"
(https://github.com/bitcoin/bitcoin/pull/33909#pullrequestreview-3518075612)
ACK b5a7a685bba312a780eddcb4a53ce2c26a937854

The PR removes incorrectness in the title of the the task "test-each-commit", which doesn't checks each commit, to make it more explicit.

This will help (starting) contributors, in my opinion, to have a better mental model of what is checked in the CI and what isn't. Therefor this is an valuable improvement and worthy of the code churn.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2570949283)
Maybe do something like this then?

```diff
--- a/test/functional/interface_rest.py
+++ b/test/functional/interface_rest.py
@@ -67,12 +67,15 @@ class RESTTest (BitcoinTestFramework):
status: int = 200,
ret_type: RetType = RetType.JSON,
query_params: Optional[dict[str, typing.Any]] = None,
+ raw_query_params: Optional[str] = None,
) -> typing.Union[http.client.HTTPResponse, bytes, str, None]:
rest_uri = '/rest' + u
...

💬 maflcko commented on pull request "depends: sqlite 3.50.4; switch to autosetup":
(https://github.com/bitcoin/bitcoin/pull/32655#issuecomment-3588516475)
The depends fallback is down again?

```
$ curl --show-headers 'https://bitcoincore.org/depends-sources/sqlite-autoconf-3500400.tar.gz'
HTTP/2 404
server: nginx
date: Fri, 28 Nov 2025 09:21:07 GMT
content-type: text/html
content-length: 146
strict-transport-security: max-age=63072000; includeSubDomains; preload

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2571010317)
> Did a tweak commit on top of [romanz@9716ff4](https://github.com/romanz/bitcoin/commit/9716ff4dd92b31fa4d97b909892f7fc8e3c82b9c): [hodlinator@0c75c32](https://github.com/hodlinator/bitcoin/commit/0c75c322a404708d82d9bbf31b70f3b036c831ba)

lgtm. Could also merge the two size/offset errors into one about just `BadPartRange`?

Seems fine to push as a commit here. It can stay separate, or be squashed, up to you.

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2571056932)
```suggestion
const auto read_tip_part{[&](auto part_offset, auto part_size){return blockman.ReadRawBlockPart(block_part, tip.GetBlockPos(), part_offset, part_size);}};
BOOST_CHECK(read_tip_part(0, std::nullopt));
```

style nit: Could reduce the verbosity in this line (and below) with a lambda.

💬 l0rinc commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2571094516)
> I must be missing something...

Uff, my sorter ignored the dots and put the smallest numbers first:
```
521546
16519598
18277917
25923656
```

Please resolve :)

✅ maflcko closed an issue: "node0 stderr bitcoind: validation.cpp:5392: void ChainstateManager::CheckBlockIndex() const: Assertion `!c->setBlockIndexCandidates.contains(const_cast<CBlockIndex*>(pindex))' failed."
(https://github.com/bitcoin/bitcoin/issues/33948)