✅ darosior closed a pull request: "spam: trick Drahtbot into rendering a scammy link"
(https://github.com/bitcoin/bitcoin/pull/32422)

💬 darosior commented on pull request "spam: trick Drahtbot into rendering a scammy link":
(https://github.com/bitcoin/bitcoin/pull/32422#issuecomment-2852185631)
Ugh it didn't even work. In this instance it did render it! https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2847530907

💬 purpleKarrot commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2074116183)
Would it be possible to put `get_directory_property(precious_variables CACHE_VARIABLES)` here inside the function scope?

💬 purpleKarrot commented on pull request "cmake: Respect user-provided configuration-specific flags":
(https://github.com/bitcoin/bitcoin/pull/32356#discussion_r2074117354)
```suggestion
string(TOUPPER "CMAKE_CXX_FLAGS_${config}" var_name)
```

💬 jonatack commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2074128855)
I don't have a strong opinion on removing the versions here; either way can probably have just one commit, e.g. "update dependency versions" or "remove versions"

🤔 glozow reviewed a pull request: "p2p: stop special-casing witness-stripped error for unconfirmed transactions"
(https://github.com/bitcoin/bitcoin/pull/32379#pullrequestreview-2816057986)
From the offline conversation, here's a [branch](https://github.com/glozow/bitcoin/tree/2025-05-rejected-parents) that drops the "don't resolve orphans with a parent in RecentRejects" logic, meaning we'll still keep an orphan of a witness stripped-transaction after this PR. This doesn't fully fix the fact that 1p1c is disrupted, because the parent is still not requested (its txid is AlreadyHave). However, keeping the orphan means we still accept the 1p1c if we receive the parent via wtxid.

Al
...

💬 hodlinator commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2074188557)
> I don't have a strong opinion on removing the versions here; either way can probably have just one commit, e.g. "update dependency versions" or "remove versions"

+1

💬 laanwj commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#issuecomment-2852352421)
> Would be good to register this new type into the Qt's meta-object system. Look at the RegisterMetaTypes() function on src/qt/bitcoin.cpp.

Good point. It's used in at least the `TransactionView::bumpedFee` signal, which might be broken now.

It could be that adding only `Q_DECLARE_METATYPE(Txid)` at the top is enough as this was enough for uint256.

🤔 Psifour reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2816051010)
ACK with 1 nit

My issue with previous PR was largely due to removal vs deprecation of config options.

💬 Psifour commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2074163691)
agreed on this nit.

It is not immediately apparent that `max_datacarrier_bytes` will be set to `MAX_OP_RETURN_RELAY` if no `-datacarriersize` flag is present.

💬 pablomartin4btc commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r2074253242)
> the for after `// Unload the wallets`...

There's the removal of the wallet from the context (`if (!RemoveWallet(context, w, /*load_on_start=*/false)) {`) using `created_wallets` - that was done when the wallet was loaded (in `LoadWalletInternal`-> `AddWallet(context, wallet);` ).

> the lines after `// Verify that there is no dangling wallet` which needs `ptr_wallet`.

At the end of the condition is returning an error (`return util::Error{error};`)

Be aware that if you remove the com
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074262054)
`CBlockIndexWorkComparator()(a, b))` means that a is strictly smaller than b.
`!CBlockIndexWorkComparator()(b, a))` means that a is smaller or or equal to b (in this case, equal means the same block index because of the [pointer address criterion](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/node/blockstorage.cpp#L165-L168)).

I think that's why sometimes one or the other is used.

In this case, it doesn't really matter because the two elements are ch
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074262125)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074262220)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074264089)
done. The suggested comment is a bit too strong for my taste: In reality the distinction doesn't matter, even if both were set nothing bad would happen, see #31835. This is more for being consistent with how it's handled elsewhere.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074264281)
done, good idea!

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074264411)
done

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074270206)
Hmm, this PR doesn't really change the essence of this:
The critical thing here is to set `nStatus` to `BLOCK_FAILED_VALID`, and this is already was the case on master.
This is irrevocable (outside of the `reconsiderblock` RPC which is a hack that doesn't really count) and it means that this block - and therefore all of its descendants - will never be connected. If there was a bug (e.g. if we could fail with `BLOCK_TIME_FUTURE` after accepting a header and then receiving the full block) this w
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2074344072)
> If that's correct, then I think:

Yes, that's correct.

> CheckBlockIndex() should be called right before releasing, but while it still has the cs_main lock, to avoid synchronization issues (effectively making CheckBlockIndex() an EXCLUSIVE_LOCKS_REQUIRED(::cs_main) method)

`CheckBlockIndex()` is called from multiple places in validation, some of which hold `cs_main`, some of which don't.
My understanding is that it must always pass if `cs_main` isn't held (because at that time, other
...

🤔 achow101 reviewed a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2816355060)
Needs a release note.

💬 achow101 commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2074352961)
In 34c3ef7160a3e54980d74426f12237a2a674e0f2 "datacarrier: deprecate startup arguments for future removal"

Perhaps this should mention that multiple outputs are allowed?