👍 l0rinc approved a pull request: "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders"
(https://github.com/bitcoin/bitcoin/pull/34022#pullrequestreview-3549208134)
Redid the change, ended up with basically the same state.
I suggest separating it into two commits to make make the change even safer to review, but I'm also fine with merging as-is (since I already did that locally).
Happy to re-ack if needed.

ACK beff6ff964c5c1b4a96af89daf6b2ddaede3464a

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3621933771)
> > Could you provide a patch for me which would go through every single transaction in mainchain history and query it both ways (full block + new RPC) and assert that they're equal?
>
> Sounds good - will it be OK to write it as a Rust CLI tool to interact with bitcoind?

https://github.com/romanz/fetch-txs can be used for equivalence testing:
```
$ cargo run --release -- 900000 900010
Finished `release` profile [optimized] target(s) in 0.11s
Running `target/release/fetch-txs 9
...

💬 maflcko commented on pull request "test: p2p: check that peer's announced starting height is remembered":
(https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3621941650)
Yeah, I also wonder what the use-case here is or was.

If it is an estimate of how long the connection is established, there is the more accurate `conntime` field. If it exists to show which peer is ahead of whom, then the existing headers-sync and block-sync p2p flow seems a better way to figure it out, which is reported via the `synced_headers`, `presynced_headers`, or `synced_blocks` fields. If it exists just for debugging the sync logic, it seems best to just log it in the debug log.


...

💬 maflcko commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3621942550)
Are you still working on this?

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2596258697)
In modern languages like Rust, yes. But not in C++, because there is no order of evaluation of the function arguments.

So `make_unique(client->m_rpc, client.release())` will lead to a nullptr deref (segfault).

I could rewrite this to use `new ProxyClient{client->m_rpc, client.release()}`, because for braced-init, the order is defined, but I am not sure if this is better, because it violates https://clang.llvm.org/extra/clang-tidy/checks/modernize/make-unique.html

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2596259276)
> I would like you to consider adding **directly** as I mentioned, as you already did some small text corrections on `results.h`

Not sure. I am not touching this line, so I'll leave as-is for now.

💬 maflcko commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621967700)
`LogPrintLevel` isn't really documented in the dev notes and the unit test name `logging_LogPrintMacrosDeprecated` seems to indicate it is deprecated. This change here seems to put a use-case to it, so it could make sense to document and test it? Though, I wonder instead of using the `LogPrintLevel` macro, it could make more sense to just add a new `LogInfo` macro that mirrors the `LogDebug` signature? I can't really see a use-case to have high-volume warnings or errors, so that they run into ra
...

💬 ajtowns commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2596296831)
Yeah. If it's not obvious, the whole thing would look something like:

```c++
auto new_peer_info = [&]() {
// lambda so that this info is only calculated if needed
const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
return strprintf("transport: %s version: %d, blocks=%d, peer=%d%s%s\n",
TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
pfrom.nVersion.load(), peer->m_starting
...

💬 Ataraxia009 commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3622056086)
> > if `-debug` is used with this implementation, it would stop rate-limiting of all category logs.
>
>
>
> Debug logs are (by design) much higher frequency than info/warning/error level logs. They have never been ratelimited, because for debugging purposes it's important to see what's happening without limitation. When running with `-debug`, the overwhelming majority of logs produced are debug logs. This PR doesn't change anything about any of that.
>
>
>
> Info logs are typically much hi
...

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596332437)
Fixed in 7802c1f9ca09b5669b8400e96baeadc238fa9df2

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596332557)
Fixed in 7802c1f9ca09b5669b8400e96baeadc238fa9df2

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596332829)
Used `util::Expected` from fa114be27b17ed32c1d9a7106f313a0df8755fa2

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596333260)
Fixed in 71c7b747091fce045dbf25e4e0cfc0e6a4ad6435

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596333791)
Fixed in 71c7b747091fce045dbf25e4e0cfc0e6a4ad6435

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2596334064)
Fixed in 71c7b747091fce045dbf25e4e0cfc0e6a4ad6435

💬 aandrews26 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3622209446)
> Are you still working on this?

Yes, I will address all comments in a patch in the next few days.

💬 adamandrews1 commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#issuecomment-3622210884)
> Are you still working on this?

Yes, I will produce an additional patch addressing all comments in the coming days.

✅ billymcbip closed a pull request: "script: Add a separate ScriptError for empty pubkeys encountered in Tapscript"
(https://github.com/bitcoin/bitcoin/pull/33961)

✅ billymcbip closed a pull request: "test: Improve STRICTENC/DERSIG unit tests in script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973)

💬 darosior commented on pull request "script: Add a separate ScriptError for empty pubkeys encountered in Tapscript":
(https://github.com/bitcoin/bitcoin/pull/33961#issuecomment-3622335233)
Force-push gone wrong @billymcbip? Or is this intentional?