👍 theuni approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2154357272)
utACK 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab

💬 maflcko commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2203902836)
https://cirrus-ci.com/task/4669206682140672?logs=ci#L3887

```
test 2024-06-30T16:34:08.989000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_contai
...

👍 ismaelsadeeq approved a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2154489545)
Code review ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-2203952477)
> https://cirrus-ci.com/task/4669206682140672?logs=ci#L3887

thanks @maflcko, will fix the C.I

🤔 mzumsande reviewed a pull request: "rpc: Avoid getchaintxstats invalid results"
(https://github.com/bitcoin/bitcoin/pull/29720#pullrequestreview-2154529465)
ACK 2342b46c451658a418f8e28e50b2ad0e5abd284d

I reviewed the code and tested the getchaintxstats output with a snapshot.

💬 maflcko commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/29398#issuecomment-2203998243)
So it only ever happened once overall?

💬 maflcko commented on issue "Slow sync ":
(https://github.com/bitcoin/bitcoin/issues/30360#issuecomment-2204000637)
> Internet seems to be ok. Restarting bitcoind doesn't help.

Can you clarify this. Does the debug log look the same after the restart?

💬 ryanofsky commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1662938949)
In commit "kernel: De-globalize signature cache" (7dd92d6fd8f0f52fcfc32739c303122c7e7630ab)

Why is this divided by 2? if this is correct, it would be good to add a comment to explain this.

🤔 ryanofsky reviewed a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2154423230)
Code review 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab, but `m_signature_cache{signature_cache_bytes / 2}` in the `ValidationCache` constructor seems like it could be a mistake?

I'd also suggest cleaning up the DEFAULT_MAX_SIG_CACHE_BYTES naming before introducing more uses of it, and adding a separate constant to avoid the need to divide by 2 most places. Maybe something like the following:

<details><summary>diff</summary>
<p>

```diff
#+begin_src diff
--- a/src/init.cpp
+++ b/src/in
...

💬 ryanofsky commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1662982423)
re: https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1658934242

> I did this now, but was not completely sure where to put the nonce generation.

For reference, this was implemented in 45e0c96e81b5f0c364af717b7cb2d9bc094372de but later dropped from the PR

💬 ryanofsky commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1662892902)
In commit "kernel: De-globalize script execution cache hasher" (468c98c8e083c68194422fc1ffc7fd77f4e0383d)

re: https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1645009465

Marked resolved because the main concern that `m_script_execution_cache_hasher` was a public member that could be modified unintentionally has been resolved. Now it is private with a const accessor.

I agree with stickies it would make code more self-documenting and modular if the private member were const and i
...

💬 maflcko commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2204115545)
Not sure what to do here, other than possibly adjusting the error message https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2029810953 or closing this as a duplicate of https://github.com/bitcoin/bitcoin/issues/30175

💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1663040526)
I think the test needs coverage for:

1) retrying broadcasts
2) sending the same tx again via sendrawtransaction
3) sending a different wtxid via sendrawtransaction
4) sending a tx with in-mempool dependencies (both that succeed and fail due to existing/missing parent)
5) rpc failing if it detects `!g_reachable_nets`

💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1662998546)
s/node1/tx_receiver/

💬 instagibbs commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1662855604)
```Suggestion
ADDRMAN_ADDRESSES = [
```

💬 maflcko commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#issuecomment-2204201089)
ACK 323ce303086d42292ed9fe7c98e8b459bdf6d1a6

💬 maflcko commented on pull request "ci: parse TEST_RUNNER_EXTRA into an array":
(https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2204283415)
ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2204306273)
Thank you for the review @ryanofsky!

Updated 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab -> c1d6e525131cb9e54bbedb79ea64e2469a77aed8 ([noGlobalScriptCache_12](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_12) -> [noGlobalScriptCache_13](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_12..noGlobalScriptCache_13))

* Applied @ryanofsky's [patch](https://github.com/bitcoin/bitcoi
...

💬 achow101 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1663112431)
In b6cea03b20bfe9486ff8f38a1c40c985d066e72d "test: add test for modififed walletprocesspsbt calls"

Removing `address_type` here and below would allow this test to run on legacy wallets so it does not need the `if self.options.descriptors` below (which would remove the conflict with #28710 and save me a rebase).

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2204312170)
I refactored `TemplateProvider` to move the lower level connection handling code into the newly introduced `Sv2Connman` from #30332.

I noticed a deadlock that was already present in the previous push (f7bf25d12d73471287c8758482014e2a429b4ff7), where if you use Bitcoin QT and start mining, an RPC call (at least to `getblockchaininfo`) will freeze and new blocks won't be submitted.