💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1585677457)
Did the necessary changes. Thanks!

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2087722633)
- Rebased to the main.
- Used the new `good_data` approach to avoid unnecessary fuzz runs.

💬 sdaftuar commented on pull request "test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py":
(https://github.com/bitcoin/bitcoin/pull/29986#discussion_r1585681716)
Oops -- I miscalculated the ancestor feerates after rewriting it this way, and the new tx still has insufficient fee. Here's what everything looks like with the change here (that is now merged, doh):

tx_unrelated_replacee: fee 31200, vsize 104
parent tx: fee 2000, vsize 147
tx_v3_child_2: fee 10400, vsize 104

tx_v3_child_3: fee 62400, vsize 157

So the total fee of the new transaction is high enough, but the ancestor feerate is still too low: (62400+2000)/304 = 211, which is less than
...

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2087734041)
`lint` seems wrong. I couldn't find any trailing whitespaces in the file.

💬 achow101 commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2087752647)
ACK 774359b4a96d2724dc70f900cb71e084a77164da

Tested on windows, the bitdeque fuzz target is indeed now available.

🚀 achow101 merged a pull request: "msvc: Compile `test\fuzz\bitdeque.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29983)

🚀 achow101 merged a pull request: "doc: update release-process.md"
(https://github.com/bitcoin/bitcoin/pull/29645)

💬 achow101 commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2087757648)
Went ahead with merging this since the vast majority of the changes here are correct and useful. It seems like there are still some questions related to the translations, but I think we can document those later when it becomes apparent that what we're actually doing diverges from the doc.

💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1585735088)
Restored that TODO in my last push. Thanks

👍 alfonsoromanz approved a pull request: "test: Assumeutxo: snapshots with less work should not be loaded"
(https://github.com/bitcoin/bitcoin/pull/29428#pullrequestreview-2032863402)
Re ACK 2f1b1eee8b8a8f546fb5975ac529c4032e46f069

💬 hernanmarino commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2087784078)
Updated to re add an incorrectly removed TODO comment.

🤔 tdb3 reviewed a pull request: "test: use sleepy wait-for-log in reindex readonly"
(https://github.com/bitcoin/bitcoin/pull/30006#pullrequestreview-2032869168)
ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c

Thank you. This should make test execution slightly more efficient and every little bit helps with many tests running in parallel (with `test_runner`).

Built and ran all functional tests (all passed). Reviewed code changes, but didn't notice anything in addition to @maflcko's comments.

👋 sr-gi's pull request is ready for review: "net: Favor peers from addrman over fetching seednodes"
(https://github.com/bitcoin/bitcoin/pull/29605)

💬 sr-gi commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2087925351)
Moving out from drat given #28016 was recently merged.

This comment is still outstanding, and I'd love some feedback on it: https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1531258847

💬 maflcko commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-2087991015)
@willcl-ark

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2087992693)
lgtm ACK 2f1b1eee8b8a8f546fb5975ac529c4032e46f069

💬 maflcko commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2088028856)
It is not wrong. No file in this repo uses Windows newlines and they are problematic when shown in git.



📝 maflcko opened a pull request: "lint: [doc] Clarify Windows line endings (CR LF) not to be used"
(https://github.com/bitcoin/bitcoin/pull/30010)
It has been this case since the linter was introduced years ago. Given a misunderstanding (https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2088028856), clarify the docs.

💬 maflcko commented on pull request "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29494#issuecomment-2088049351)
Rebased (should be trivial to re-ACK)

💬 maflcko commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#discussion_r1585942374)
```suggestion
LogInfo("Couldn't connect to peers from addrman after %d seconds. Adding seednode (%s) to addrfetch\n", ADD_NEXT_SEEDNODE.count(), seed);
```

nit: For new code, it could make sense to use the clear and non-deprecated alias here.