💬 MarcoFalke commented on issue "Issue with `wallet_importdescriptors.py --descriptors` under valgrind":
(https://github.com/bitcoin/bitcoin/issues/27229#issuecomment-1463817073)
I don't think it currently works in the current form.

💬 Sjors commented on pull request "assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/15606#discussion_r1132390967)
It would be useful to have information about dbcache allocation (and usage) here.

💬 MarcoFalke commented on issue "Run functional tests from make check":
(https://github.com/bitcoin/bitcoin/issues/18816#issuecomment-1463821930)
Given that we will switch from cmake to make, it could make sense to wait for that and then only implement this in cmake?

🚀 fanquake merged a pull request: "Use string interpolation for default value of -listen"
(https://github.com/bitcoin/bitcoin/pull/27232)

💬 fanquake commented on pull request "test: add coverage for sigop limit policy (`-bytespersigop` setting)":
(https://github.com/bitcoin/bitcoin/pull/27171#issuecomment-1463841962)
@theStack could open a followup addressing anything here?

> (Doesn't need to be in this PR) it could be worth checking that this vsize calculation is used for anc/desc limits as well?

> for reference, usually fAccurate is set for witness scripts sigop counts, but omitting the k and n in the CMS imitates fAccurate=false.

💬 MarcoFalke commented on pull request "test: add coverage for sigop limit policy (`-bytespersigop` setting)":
(https://github.com/bitcoin/bitcoin/pull/27171#issuecomment-1463846459)
My comment was only meant for reviewers. A unit test may be more appropriate if someone wants to check the behavior of the sigop counting function itself.

💬 brunoerg commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1463851769)
I agree on having test coverage for it with a network arg, something like:
```diff
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 3d39fb47d..6928c1211 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -331,11 +331,23 @@ class NetTest(BitcoinTestFramework):

def test_getaddrmaninfo(self):
self.log.info("Test getaddrmaninfo")
- # current ipv4 count in node 1's Addrman: new 1, tried 1
self.log.debug("Tes
...

📝 MuhammadFathurNurRizky opened a pull request: "Year update"
(https://github.com/bitcoin/bitcoin/pull/27240)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 MuhammadFathurNurRizky commented on pull request "Year update":
(https://github.com/bitcoin/bitcoin/pull/27240#issuecomment-1463856637)
Hello

✅ fanquake closed a pull request: "Year update"
(https://github.com/bitcoin/bitcoin/pull/27240)

💬 fanquake commented on pull request "Year update":
(https://github.com/bitcoin/bitcoin/pull/27240#issuecomment-1463857356)
Thanks. However these changes are done using a script, only at certain times.

💬 fanquake commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1463871792)
Concept ACK

💬 Sjors commented on pull request "assumeutxo: keep cache when flushing snapshot (#17487 followup)":
(https://github.com/bitcoin/bitcoin/pull/27008#issuecomment-1463883934)
From my testing of #15606 - without this PR - I get the impression that flushing isn't working, specifically that RAM is not freed up. It could just be confusion on my end from rebase hell, but I'd like to make sure we're not obscuring a bug by merging this.

💬 fanquake commented on pull request "refactor: Consistenly use context args over gArgs in node/interfaces":
(https://github.com/bitcoin/bitcoin/pull/27239#issuecomment-1463886234)
Concept ACK

💬 furszy commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1132459068)
yeah good, fixed.
also, pushed an early return if the sum of the not accepted groups decreases the available amount below the target (we simply don't have enough balance after applying the filters, so no need to run the selection algos).

👋 dergoegge's pull request is ready for review: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)

💬 furszy commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#issuecomment-1463907731)
Thanks @S3RK!, feedback tackled.

We now check, and return early, if there is no enough available balance after filtering groups and before running the selection algos.

💬 MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1132473107)
```suggestion
uint64_t assumed_chain_bytes{chainparams.AssumedBlockchainSize() * 1024 * 1024 * 1024};
```

Instead of "size", use "bytes", like in other places, for example `additional_bytes_needed`?

💬 MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#issuecomment-1463915543)
Should be trivial to add a test, no?

💬 furszy commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1132487327)
tiny nit:
Extra whitespace space.

💬 furszy commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1132491161)
This is not needed. With line 17 is enough, the include is done in the cpp file.