💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2856025715)
> > I added the `if()` guard because I heard from [@hebasto](https://github.com/hebasto) that we call the toolchain multiple times, and the `if` guard therefore avoids adding the path repeatedly.
>
> I think this is half-true. I noticed by adding prints to the cmake file that it was called multiple times, but each time CMAKE_PREFIX_PATH was always set back to the original value, so I don't think the `if` guard did anything.

I'd avoid making assumptions about undocumented `CMAKE_PREFIX_PATH`-re
...

💬 furszy commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856093736)
> Why in 5 years btc tx should concur for space inside block with another data? blockchain is about bitcoin not other data, if you people want to store data it should be expensive and hard, or use side chains like liquid etc.

I'm somewhat puzzled by your comment. Storing data on the blockchain in its current form is not particularly difficult—that’s precisely what this discussion is about. It can be done in various ways, some more harmful than others.

As you noted, if the goal is to make d
...

💬 hebasto commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#issuecomment-2856105202)
Rebased due to a conflict with the merged bitcoin/bitcoin#32086.

💬 maflcko commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#discussion_r2076388963)
```suggestion
Please note that package availability might depend on your arch+OS you are building on.
```

💬 achow101 commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2076408536)
I believe the plan is to rewrite all of the tests into this style over the course of a couple PRs. So this parameter is here to allow for the future testing of SFFO behavior.

> Also, you are expecting group.GetSelectionAmount() to be equal to tx.vout[0].nValue with this parameter right?

Only when it is false. The purpose is to have `group.GetSelectionAmount()` be eqaul to `amount`, so `nValue` is increased by the amount in fees so that they match after fees are deducted.

💬 hebasto commented on pull request "depends: Switch from multilib to platform-specific toolchains":
(https://github.com/bitcoin/bitcoin/pull/32162#discussion_r2076413998)
Thanks! Fixed.

💬 Craig-k-p commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856227822)
RIP core.

💬 w0xlt commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076416803)
```suggestion
assert(wallet_instance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
```

💬 achow101 commented on pull request "scripted-diff: adapt script error constant names in feature_taproot.py":
(https://github.com/bitcoin/bitcoin/pull/32415#issuecomment-2856231027)
ACK b5f580c580257d28d295cae3f787b55eb1863f16

🚀 achow101 merged a pull request: "Refactor BnB tests"
(https://github.com/bitcoin/bitcoin/pull/29532)

💬 w0xlt commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076425739)
I agree.

💬 brunoerg commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076427518)
Agreed. Otherwise, #28710 would have to drop it.

👍 brunoerg approved a pull request: "fuzz: Remove unused TimeoutExpired catch in fuzz runner"
(https://github.com/bitcoin/bitcoin/pull/32388#pullrequestreview-2819842011)
code review ACK fa4804009ceba96926edd7f55ea22442ebdc665d

🚀 achow101 merged a pull request: "scripted-diff: adapt script error constant names in feature_taproot.py"
(https://github.com/bitcoin/bitcoin/pull/32415)

💬 esomore commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856261607)
Now that Bitcoin Knots adoption is growing, it’s time to harden policy—enforce secp256k1 curve checks on bare pub keys to prevent invalid-key UTXO spam.

🤔 w0xlt reviewed a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2819848971)
ACK https://github.com/bitcoin/bitcoin/pull/28710/commits/be3c13a22fa8d49fe15cde941e8030e1cd2b522f with 2 non-blocking nits commented earlier.

Regarding the removed RPC functons, I think less is better in this case, and unless I'm missing something, the cases mentioned in https://github.com/bitcoin/bitcoin/issues/30175 can be performed by `importdescriptors`.

📝 maflcko opened a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430)
Currently mocktime is written to a global, which may leak between sub-tests (albeit some tests try to reset the mocktime on a best-effort basis). Also, when advancing it, one has to keep a counter variable around.

Fix both issues by a new `ElapseTime` helper, which resets the mocktime once it goes out of scope. Also, it has a method to advance the mocktime by a delta.

💬 RandyMcMillan commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856341229)
ACK cd7872ca543d31d20f419fd2203138b8301c2e68

💬 pokrovskyy commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856387463)
We all know we have Ordinals protocol that utilizes Taproot witness to store arbitrary data on-chain. Effectively permanently. And literally of any size (even beyond the block limits via chunking techniques). Regardless of the outcome of this conversation, this pathway will remain. Moreover, size-wise / fee-wise keeping bigger data in witness is 4x more blockspace-efficient and cheaper. So there's no incentive to choose OP_RETURN over witness data for significantly bigger data so should not incr
...

💬 achow101 commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2856433009)
In 8f2078af6a55448c003b3f7f3021955fbb351caa "miner: timelock coinbase transactions"

I'm having a hard time understanding why `rbf_tests.cpp` and `mempool_package_rbf.py` are changed? These are not hardcoded txid changes. Rather the specific changes suggest to me that somehow the semantics of rbf have somehow changed? But that doesn't make sense if the only changes are to the coinbase transaction, why would that affect mempool policy?