🤔 w0xlt reviewed a pull request: "log: always print initial script verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3255852704)
reACK https://github.com/bitcoin/bitcoin/pull/33336/commits/010cf81407c0df8de766fd2a116463d180f25f33

💬 ariard commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3322176062)
Concept ACK.

To give a single data point, it took 3.5 years to go from an initial proposal about
`-mempoolfullrbf` during the release cycle of [v22.0](https://gnusha.org/pi/bitcoindev/CALZpt+F2b3tdu1+kLZiBPCH2O-pDzZytoRFtX6X0a8UX4OBrDQ@mail.gmail.com/) to a release completely deprecating
the ability for a full node operator to police its processed transactions with the opt-in
RBF policy (v[29.0](https://github.com/bitcoin/bitcoin/pull/30592)). If there is no consensus, it's wiser to take t
...

🤔 pablomartin4btc reviewed a pull request: "doc: remove unrelated `bitcoin-wallet` binary from `libbitcoin_ipc` description"
(https://github.com/bitcoin/bitcoin/pull/33459#pullrequestreview-3255954505)
ACK fbde8d9a81d82e53933fe45e36d3a70206a48e0e

I agree that the tool is unrelated to IPC/ multiprocess, so removing it from the list of binaries that use libbitcoin_ipc is appropriate. As an offline wallet utility, it doesn’t provide full “management” like a running wallet RPC (no sending, no RPC-based operations); its capabilities are basically limited to creating wallets, showing wallet details, and importing/exporting wallet data.

💬 ajtowns commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3322333174)
> Is the argument that poor compact block reconstruction rates leads to miner centralization overblown then if the loss in revenue due to orphans is dwarfed by the increasing difficulty? I thought this argument was one of the reasons for both uncapping `OP_RETURN` limits and for allowing sub-1sat/vb relay?

The comparison is that losing $1.70 out of every $1000 in revenue is effectively just noise or a rounding error in an environment where you're getting $195 swings due to other factors, so i
...

💬 dergoegge commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#issuecomment-3322339038)
@achow101 @fanquake @glozow @hebasto @ryanofsky This has two good ACKs and no outstanding review comments. Would be great to hear what's missing here.

👋 enirox001's pull request is ready for review: "rpc: Fix dumptxoutset rollback with competing forks"
(https://github.com/bitcoin/bitcoin/pull/33444)

💬 enirox001 commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3322397212)
Opened the PR #33444 to address this issue

💬 ajtowns commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#issuecomment-3322415437)
Added the test after slightly cleaning it up

💬 Eunovo commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371206072)
Alright

💬 Eunovo commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371206290)
Alright

🤔 ajtowns reviewed a pull request: "log: always print initial script verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3256339146)
Some possible nits

💬 ajtowns commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371245648)
switch the check to:

```c++
if (AssumedValidBlock().IsNull()) {
script_check_reason = "assumevalid=0";
} else {
// ... long code block ...
}
```

?

💬 ajtowns commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371233054)
```c++
} else if (pindex->nHeight > it->second.nHeight) {
script_check_reason = "block height above ..";
} else if (it->second.GetAncestor(pindex->nHeight) != pindex) {
script_check_Reason = "block not part of av chain";
...
```

💬 ajtowns commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371229366)
Perhaps:

```c++
constexpr auto TOO_RECENT_FOR_ASSUMEVALID = std::chrono::days{14})};
...
} else if (GBPET(...) <= count_seconds(TOO_RECENT_FOR_ASSUME_VALID)) {
```

💬 ajtowns commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371247053)
Separate text with a blank line rather than indenting?

💬 ajtowns commented on pull request "log: always print initial script verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2371280673)
consider setting `const bool fScriptChecks = (script_check_reason != nullptr);` here, and leaving the following code unchanged?

💬 ajtowns commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2371296279)
Perhaps also noting that rate limiting does not apply to LogDebug and LogTrace would help encourage people to use those macros for noisy logs, rather than sticking with INFO level but disabling rate limiting.

💬 Sjors commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3322771214)
As I try newer guix time machine commits, new cans of worms keep being opened. E.g. when testing the proposed fix for https://codeberg.org/guix/guix/issues/1257 I ran into yet more packages failing to build, probably related to this: https://codeberg.org/guix/guix/issues/928. Hopefully one day there'll be a master commit that just works(tm).

Until then, it's probably better than we recommend users to use one substitute. Thanks for the hint @trevarj.

Going back and forth with the time machi
...

💬 maflcko commented on issue "cli:passing non-integers to datacarriersize":
(https://github.com/bitcoin/bitcoin/issues/33460#issuecomment-3322788080)
This is a known issue and should affect almost all args. See also https://github.com/bitcoin/bitcoin/issues/22978

💬 Kruwed commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3322791530)
>This specific PR seeks clarity in the datacarrier options' future availability, in favor of removing the deprecation, as the deprecation designation definition is unclear, potentially incorrect (if the definition is will be removed), and causes uncertainty for Bitcoin Core users around timing.

Why shouldn't the datacarrier options be removed? My earlier question remains unaddressed:

>>Why should Bitcoin Core support a config option to perform targeted censorship of large OP_RETURNs?

@S
...