💬 instagibbs commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#issuecomment-2203545930)
reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746

reviewed via "git range-diff master a6821a0 926b8e3"

💬 spacebear21 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2203628213)
Thanks @willcl-ark for picking this up! Confirmed this fixes the CHECK_NONFATAL abort when a witness signature is invalid.

tACK f57f68ba95fc8778fe2dc755da1e631fe60592c4

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203675621)

> I think the last commit [064a401](https://github.com/bitcoin/bitcoin/commit/064a401e10f2b2da8f31f682929431d06c671d93) is unnecessary scope creep for this PR, and although it looks safe to me, I am not 100% confident I am grasping the full extent of the implications, and I think a separate PR would've been more suitable.

Yeah, on second thought, I agree. My bad. I'd also feel more comfortable ACKing without it.

@TheCharlatan want to pop that one off?

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2203689268)
Force-pushed changing more sizes, I updated the PR description with the changes.

💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2203785402)
Updated 45e0c96e81b5f0c364af717b7cb2d9bc094372de -> 7dd92d6fd8f0f52fcfc32739c303122c7e7630ab ([noGlobalScriptCache_11](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_11) -> [noGlobalScriptCache_12](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_11..noGlobalScriptCache_12))

* Dropped the last commit as requested by @stickies-v and @theuni

👍 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 = [
```