💬 TheCharlatan commented on pull request "doc: Show how less noisy clang-tidy output can be achieved":
(https://github.com/bitcoin/bitcoin/pull/27205#discussion_r1132379578)
I don't mind either way. I added a separate explainer, because it felt a bit too information-dense otherwise. Also, I only have bear version 3 to test. Does bear version 2 also accept `--config`?
(https://github.com/bitcoin/bitcoin/pull/27205#discussion_r1132379578)
I don't mind either way. I added a separate explainer, because it felt a bit too information-dense otherwise. Also, I only have bear version 3 to test. Does bear version 2 also accept `--config`?
🚀 fanquake merged a pull request: "test: add coverage for sigop limit policy (`-bytespersigop` setting)"
(https://github.com/bitcoin/bitcoin/pull/27171)
(https://github.com/bitcoin/bitcoin/pull/27171)
💬 fanquake commented on issue "Issue with `wallet_importdescriptors.py --descriptors` under valgrind":
(https://github.com/bitcoin/bitcoin/issues/27229#issuecomment-1463812501)
> Maybe re-try after https://github.com/bitcoin/bitcoin/pull/27199 ?
Running that on master at the moment.
(https://github.com/bitcoin/bitcoin/issues/27229#issuecomment-1463812501)
> Maybe re-try after https://github.com/bitcoin/bitcoin/pull/27199 ?
Running that on master at the moment.
💬 MarcoFalke commented on pull request "doc: Show how less noisy clang-tidy output can be achieved":
(https://github.com/bitcoin/bitcoin/pull/27205#discussion_r1132385355)
Seems fine to drop version 2
(https://github.com/bitcoin/bitcoin/pull/27205#discussion_r1132385355)
Seems fine to drop version 2
✅ fanquake closed a pull request: "Scale network graph based on time interval"
(https://github.com/bitcoin-core/gui/pull/539)
(https://github.com/bitcoin-core/gui/pull/539)
💬 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.
(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.
(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?
(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)
(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.
(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.
(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
...
(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
...
(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
(https://github.com/bitcoin/bitcoin/pull/27240#issuecomment-1463856637)
Hello
✅ fanquake closed a pull request: "Year update"
(https://github.com/bitcoin/bitcoin/pull/27240)
(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.
(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
(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.
(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
(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).
(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).