✅ fanquake closed an issue: "f"
(https://github.com/bitcoin/bitcoin/issues/32518)

🚀 fanquake merged a pull request: "doc: Improve `dependencies.md`"
(https://github.com/bitcoin/bitcoin/pull/31895)

💬 Sjors commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884350379)
@szarka the extra pool is capped at 100kb regardless of the number of transactions, so any filtered content over 100kb would overflow it.

It's (unfortunately) unclear from your log example what these 87 transactions were. If they were all RBF replacements, then this PR wouldn't hurt that. If they're all inscriptions, then it would.

> Seems like including things that are rejected for policy reasons is useful.

In principle yes. The problem is that when we decide a transaction is invalid f
...

💬 szarka commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884363317)
> In any case I'd love to see @0xB10C's data. I'll keep this in draft.

I think maybe [this _Delving_ post](https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052) is what @glozow was referring to?

💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091545479)
nit: Not sure about making this optional out of convenience and then write the rule in the description string.

It would be better to just pass down a `std::optionl<RPCResult>` (or `std::vector<RPCResult>`) here, and then use `Cat()` to concatenate it?

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091547152)
I spose I'd ask the question the other way round, why not the exception? It would be an exceptional circumstance if MSBuild could not be found.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091547757)
Will try without.

💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091548060)
```suggestion
if (is_change_func ) {
out.pushKV("ischange", is_change_func(txout));
```

nit: Seems more user friendly to not make the field optional and instead return a boolean?

💬 stickies-v commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2091555297)
Oh right, they have the same bit representation, so underflowing here doesn't mean anything for serialization (and vice versa). Thanks - can be marked as resolved!

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091558808)
I mean, did GHA document handling exceptions in PowerShell scripts?

🤔 furszy reviewed a pull request: "qt, wallet: Convert uint256 to Txid"
(https://github.com/bitcoin/bitcoin/pull/32238#pullrequestreview-2844335248)
Code review ACK 0671d66a8ee07584fab6f1ddaaa188c6a4ac25c1

Happy rebase for everyone.

💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091584150)
I started with this but then playing with the feature I made it optional because if you receive a payment you'd see a tx with a bunch of outputs that all say `ischange: false`. I felt like that was confusing because you didn't create this tx and that might imply that there is some way to detect a change output in someone else's tx.

📝 instagibbs converted_to_draft a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage"
(https://github.com/bitcoin/bitcoin/pull/32516)
`DisconnectedBlockTransactions::LimitMemoryUsage()` has unit test coverage, but the default value end to end doesn't have coverage.

This test adds exercised coverage of memory limiting of the disconnect pool, and some basic behavior sanity checks.

I didn't assert the exact order of pruning, but can if desired.

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#issuecomment-2884446941)
I don't think I'm actually hitting that particular limit, getting weird results when looking closer, draft for now while I look

💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091591988)
Not sure what you mean here -- `help gettransaction` doesn't list this at all, it just tells the user to check `decoderawtransaction`

📝 maflcko opened a pull request: "ci: Enable feature_init and wallet_reorgsrestore in valgrind task"
(https://github.com/bitcoin/bitcoin/pull/32519)
The `fork()` isn't needed and in fact causes issues, so just avoid it and run the valgrind process directly in the CI tasks.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091600215)
No, this [doc](https://docs.github.com/en/actions/sharing-automations/creating-actions/setting-exit-codes-for-actions) just says use any exit code other than 0 (which throw does do anyway).

I've tested throw works here: https://github.com/m3dwards/bitcoin/actions/runs/15050358923/job/42303402427

All that said, I'm really not wedded to it and happy to change to a simple `exit 1` if you prefer.

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2884478736)
Rebased after #31895.

📝 Sjors converted_to_draft a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)
Have depends make libmultiprocess by default. This PR causes the following behavior changes:

1. **bitcoin-node and bitcoin-gui binaries are included in releases**, due to `ENABLE_IPC` option being switched on by default in depends builds
2. `ENABLE_IPC` is also switched on by default in non-depends builds
3. Various changes to CI: switching on `ENABLE_IPC` on in most configurations and using `bitcoin-node` binary for functional tests in two of them.
4. The `bitcoin-node` and `bitcoin-gui`
...

💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091630238)
Hmm, looks like you are right. Though, that doesn't seem very machine readable. It is a bit unrelated, but I can try to fix it, if you don't want to include it here.