✅ fanquake closed an issue: "32-bit Linux: build flags lost with depends & overriden CC(X)"
(https://github.com/bitcoin/bitcoin/issues/28096)

💬 EthanHeilman commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2863344633)
@AlexSQY I see your point. Let me see if there is anything that can be done to ensure it is never the case that `OP_CAT = False && DISCOURAGE_OP_CAT = False` since that should never happen outside of tests.

The behavior is what you see in that test is the expected behavior for `DISCOURAGE_OP_CAT = False` and `OP_CAT = False`,
since `OP_CAT = False` the CAT opcode is an OP_SUCCESS so it returns success (`OK`).

@0xBEEFCAF3 What do you think?

💬 marcofleon commented on pull request "test: added fuzz coverage for consensus/merkle.cpp":
(https://github.com/bitcoin/bitcoin/pull/32243#discussion_r2079888041)
I think it would be better to consume from a range here and below. `MAX_SCRIPT_SIZE` is large (10000) so the fuzzer ends up consuming the remaining bytes in the data provider at this line.

💬 stickies-v commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2863362683)
re-ACK 0671d66a8ee07584fab6f1ddaaa188c6a4ac25c1, no changes since 65fcfbb2b38bef20a58daa6c828c51890180611d except rebase.

👍 ryanofsky approved a pull request: "cmake: Respect user-provided configuration-specific flags"
(https://github.com/bitcoin/bitcoin/pull/32356#pullrequestreview-2825385905)
Code review ACK edde96376a2961dec3730331b3d171ddf972589f

💬 ryanofsky commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2079862300)
In commit "cmake: Respect user-provided configuration-specific flags" (edde96376a2961dec3730331b3d171ddf972589f)

Would be good for the comment here to note that this needs to be called before the project() command otherwise the list will include variables not actually set by the user. (I wouldn't have known that unless I saw the discussion at https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2075396465)

👍 laanwj approved a pull request: "test: refactor: negate signature-s using libsecp256k1"
(https://github.com/bitcoin/bitcoin/pull/32436#pullrequestreview-2825454444)
Code review ACK 1ee698fde2e8652d8eb083749edb8029c003b8db

👍 laanwj approved a pull request: "contrib: remove bdb exception from FORTIFY check"
(https://github.com/bitcoin/bitcoin/pull/32448#pullrequestreview-2825462550)
Code review ACK f9dfe8d5e0d3f628659702ab781b7919505c829f

🤔 ismaelsadeeq reviewed a pull request: "doc: warn that CheckBlock() underestimates sigops"
(https://github.com/bitcoin/bitcoin/pull/31624#pullrequestreview-2825308443)
ACK 0ac19a98f329fe8952f394509a7a29775ab805b0 with a nit and another suggestion


The size limit check above the sigops check also underestimates the block size because it does not include the witness.

💬 ismaelsadeeq commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2079816983)
nit: maybe be specific.
```suggestion
// This underestimates the number of sigops, because unlike ConnectBlock it does not count the witness sigops and p2sh sigops:

```

💬 ismaelsadeeq commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2079904329)
I think it's okay to have this check here to catch those violations early.

🚀 ryanofsky merged a pull request: "cmake: Respect user-provided configuration-specific flags"
(https://github.com/bitcoin/bitcoin/pull/32356)

💬 ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-2863404137)
Friendly ping @glozow @willcl-ark @instagibbs

💬 laanwj commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#issuecomment-2863424559)
> When someone builds from a tarball that is more than a year old, this will use the current year instead of the year of the release. Is that a valid use case?

Not only with tarballs, it's true for any non-guix build, also from an old git commit.

💬 instagibbs commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-2863430236)
Is this something we need to support? In a sense the current mechanism is honestly saying it doesn't have enough data. If you don't actually know what level of fees are required to get in a block, you may miss the fact that, f.e., miners are not mining anything 2x higher than your floating minfee.

I think this circles back to what the end goal of this RPC is, A one stop shop for best guess of feerates needed?

I haven't thought about this much in years, so I'd be curious to hear what you th
...

🤔 mzumsande reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2825526715)
Concept / Approach ACK, wrote the reason in [the other PR](https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855558032)

💬 ryanofsky commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#discussion_r2079946510)
In commit "build: let CMake determine the year" (dfeac76b951fc2d28e0832b176f3c8ad595697f1)

Doesn't this make the build nondeterministic?

💬 laanwj commented on pull request "build: let CMake determine the year":
(https://github.com/bitcoin/bitcoin/pull/32445#discussion_r2079949509)
No; SOURCE_DATE_EPOCH, which is set by the guix build, is respected: https://github.com/bitcoin/bitcoin/pull/32445#pullrequestreview-2824986604

💬 hebasto commented on issue "build: cmake --install fails after --target deploy":
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2863466359)
From CMake [docs](https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_INSTALL_ALL_DEPENDENCY.html):
> By default, the `install` target depends on the `all` target. This has the effect, that when `make install` is invoked or `INSTALL` is built, first the `all` target is built, then the installation starts.

[On the other hand](https://cmake.org/cmake/help/latest/manual/cmake.1.html#install-a-project), the `cmake --install <dir>` command:
> ... may be used _after_ building a project to run ins
...

💬 rstmsn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2863484560)
The OP_RETURN `-datacarriersize` limit is 100% effective in cases where miners use it to filter their local mempools & inform template construction, prior to successfully mining a block.

This viable use case would be harmed as a result of this PR. On that basis, firm NACK.