π¬ sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2928370752)
If people are interested in a much larger scale comparison between the old and the new algorithm:
```
# build bench_bitcoin
wget https://bitcoin.sipa.be/clusters/clusters.tgz
tar -xzf clusters.tgz
for F in clusters_sim2023_large clusters_medium clusters_spanning clusters_bipartite; do time ./build/bin/bench_bitcoin --filter="LinearizeDataSet" <~/$F >~/$F.out; done
tar -czf clusters.out.tgz clusters_*.out
```
And publish the resulting clusters.out.tgz file somewhere. The whole process
...
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2928370752)
If people are interested in a much larger scale comparison between the old and the new algorithm:
```
# build bench_bitcoin
wget https://bitcoin.sipa.be/clusters/clusters.tgz
tar -xzf clusters.tgz
for F in clusters_sim2023_large clusters_medium clusters_spanning clusters_bipartite; do time ./build/bin/bench_bitcoin --filter="LinearizeDataSet" <~/$F >~/$F.out; done
tar -czf clusters.out.tgz clusters_*.out
```
And publish the resulting clusters.out.tgz file somewhere. The whole process
...
π¬ maflcko commented on pull request "fee estimate test: fix #31944 by handling a legitimate scenario that β¦":
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2120212961)
@tnndbtc I am wondering, are you using an "AI agent" or LLM to generate the iterations on this pull request?
(https://github.com/bitcoin/bitcoin/pull/32615#discussion_r2120212961)
@tnndbtc I am wondering, are you using an "AI agent" or LLM to generate the iterations on this pull request?
π¬ maflcko commented on pull request "wip: Split fuzz binary (take 2)":
(https://github.com/bitcoin/bitcoin/pull/30882#issuecomment-2929138831)
cc @dergoegge . the issue https://github.com/bitcoin/bitcoin/issues/28971 is still open, so I guess this pull can be closed or rebased?
(https://github.com/bitcoin/bitcoin/pull/30882#issuecomment-2929138831)
cc @dergoegge . the issue https://github.com/bitcoin/bitcoin/issues/28971 is still open, so I guess this pull can be closed or rebased?
π¬ maflcko commented on pull request "wallet: Track no-longer-spendable TXOs separately":
(https://github.com/bitcoin/bitcoin/pull/27865#discussion_r2120269086)
βwith it's defaultβ -> βwith its defaultβ [βit'sβ is a contraction of βit is,β but here the possessive βitsβ is required]
(https://github.com/bitcoin/bitcoin/pull/27865#discussion_r2120269086)
βwith it's defaultβ -> βwith its defaultβ [βit'sβ is a contraction of βit is,β but here the possessive βitsβ is required]
π¬ maflcko commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2120285755)
noticable -> noticeable [correct spelling]
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2120285755)
noticable -> noticeable [correct spelling]
π¬ maflcko commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2111188374)
nit: new code should put the `}` before the `else if`
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2111188374)
nit: new code should put the `}` before the `else if`
π¬ maflcko commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2120302899)
not sure why this was marked resolved?
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2120302899)
not sure why this was marked resolved?
π¬ l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120449098)
> not sure about the structured bindings
That's what helped me in [noticing](https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2108817538) that we weren't checking `request_more`
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120449098)
> not sure about the structured bindings
That's what helped me in [noticing](https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2108817538) that we weren't checking `request_more`
π¬ l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120449675)
* they're simpler/less specific
* they're shorter (matters for these long lines)
* you're already changing them in other places
i.e. what's the point of keeping them as vectors when we don't actually need so much power?
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120449675)
* they're simpler/less specific
* they're shorter (matters for these long lines)
* you're already changing them in other places
i.e. what's the point of keeping them as vectors when we don't actually need so much power?
π¬ l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120450540)
Based on the [Sonarcloud](https://corecheck.dev/bitcoin/bitcoin/pulls/32579) recommendations I eagerly checked which other constructors can be explicit, but [exploring](https://cplusplus.com/forum/general/168292/) your question in more detail it seems there's barely any advantage in explicit default constructors.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120450540)
Based on the [Sonarcloud](https://corecheck.dev/bitcoin/bitcoin/pulls/32579) recommendations I eagerly checked which other constructors can be explicit, but [exploring](https://cplusplus.com/forum/general/168292/) your question in more detail it seems there's barely any advantage in explicit default constructors.
π¬ l0rinc commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120450728)
It seems to be a mix indeed:
```bash
% find . -name "*.h" | grep '_' | wc -l
185
```
vs
```bash
% find . -name "*.h" | grep '-' | wc -l
39
```
and most of those are from the recent `ipc` changes:
```bash
% find . -name "*.h" | grep '-' | grep -v ipc | wc -l
11
```
it's just a nit from my part, but standard lib also uses underscores mostly (e.g. `string_view`) and in many [regexes](https://www.w3schools.com/jsref/jsref_regexp_wordchar.asp) the `\w`, i
...
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120450728)
It seems to be a mix indeed:
```bash
% find . -name "*.h" | grep '_' | wc -l
185
```
vs
```bash
% find . -name "*.h" | grep '-' | wc -l
39
```
and most of those are from the recent `ipc` changes:
```bash
% find . -name "*.h" | grep '-' | grep -v ipc | wc -l
11
```
it's just a nit from my part, but standard lib also uses underscores mostly (e.g. `string_view`) and in many [regexes](https://www.w3schools.com/jsref/jsref_regexp_wordchar.asp) the `\w`, i
...
β
dergoegge closed a pull request: "wip: Split fuzz binary (take 2)"
(https://github.com/bitcoin/bitcoin/pull/30882)
(https://github.com/bitcoin/bitcoin/pull/30882)
π¬ fanquake commented on pull request "[28.x] Backport #31407":
(https://github.com/bitcoin/bitcoin/pull/32563#discussion_r2120548419)
Thanks, is should be fixed now. It's unrelated to #31161, was a bad cherry-pick fixup by me.
(https://github.com/bitcoin/bitcoin/pull/32563#discussion_r2120548419)
Thanks, is should be fixed now. It's unrelated to #31161, was a bad cherry-pick fixup by me.
π fanquake's pull request is ready for review: "[28.x] Backport #31407"
(https://github.com/bitcoin/bitcoin/pull/32563)
(https://github.com/bitcoin/bitcoin/pull/32563)
π willcl-ark approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2887515593)
ACK a189d636184b1c28fa4a325b56c1fab8f44527b1
I would have also had a slight preference for "removing the (ineffective) knob" a.k.a the previous iteration of this PR, but removing the limited default along with a notice of intended deprecation works for me too.
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2887515593)
ACK a189d636184b1c28fa4a325b56c1fab8f44527b1
I would have also had a slight preference for "removing the (ineffective) knob" a.k.a the previous iteration of this PR, but removing the limited default along with a notice of intended deprecation works for me too.
π¬ rleed commented on issue "Add config option to set external P2P port to facilitate incoming connections":
(https://github.com/bitcoin/bitcoin/issues/32657#issuecomment-2929709339)
> I see. I believe this already works, you can add a port number to the `externalip=` argument.
Wonderful! I had tried this before, but I had overlooked a detail in my configuration....my bad. Now it's working! Thank you!
(https://github.com/bitcoin/bitcoin/issues/32657#issuecomment-2929709339)
> I see. I believe this already works, you can add a port number to the `externalip=` argument.
Wonderful! I had tried this before, but I had overlooked a detail in my configuration....my bad. Now it's working! Thank you!
π¬ Sjors commented on pull request "init: deprecate `-blockmaxweight` startup option":
(https://github.com/bitcoin/bitcoin/pull/32654#issuecomment-2929711294)
I don't think we should recommend `-blockreservedweight` as an alternative to `-blockmaxweight`, as that just adds confusion.
At this point `-blockmaxweight` seems useless in production, but it appears to be useful in test networks and our owns tests. We could hide the option under `-help-debug` in order to unclutter the `--help` output. But I wouldn't drop it.
(https://github.com/bitcoin/bitcoin/pull/32654#issuecomment-2929711294)
I don't think we should recommend `-blockreservedweight` as an alternative to `-blockmaxweight`, as that just adds confusion.
At this point `-blockmaxweight` seems useless in production, but it appears to be useful in test networks and our owns tests. We could hide the option under `-help-debug` in order to unclutter the `--help` output. But I wouldn't drop it.
π¬ fanquake commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2120625826)
Actually, looking at this again, with 0.16.6 we can just use `has_nx`, as that does include both checks?
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2120625826)
Actually, looking at this again, with 0.16.6 we can just use `has_nx`, as that does include both checks?
π¬ fanquake commented on issue "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files":
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-2929790543)
What's the actual blocker to just making this change?
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-2929790543)
What's the actual blocker to just making this change?
β
maflcko closed an issue: "Add config option to set external P2P port to facilitate incoming connections"
(https://github.com/bitcoin/bitcoin/issues/32657)
(https://github.com/bitcoin/bitcoin/issues/32657)