π¬ jsarenik commented on issue "Zero output not cleared":
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3233911050)
The linked transactions ([...4628](https://mempool.space/signet/tx/a7b388a8f48350f92f8987d45409723bf329519816eba808de59ad6ebe974628) and [...34f5](https://mempool.space/signet/tx/92246e21897116e7e7b9afc62034caaadb75ae61c441c52df4a66dbdd9e234f5)) in annotated raw:
```
VERSION: 03000000
FLAG: 0001
INPUTS: 01
0c571f65a11a383eb53896d93be77cccaa979234a7c93b1cbde63d82fde01b57 01000000 00 00000000 #I-0
OUTPUTS: 03 #0..2
e803000000000000 22 5120db7b995709b105a12719e1b4d92ab4bb616805fe0e2aee2db4b86efb
...
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3233911050)
The linked transactions ([...4628](https://mempool.space/signet/tx/a7b388a8f48350f92f8987d45409723bf329519816eba808de59ad6ebe974628) and [...34f5](https://mempool.space/signet/tx/92246e21897116e7e7b9afc62034caaadb75ae61c441c52df4a66dbdd9e234f5)) in annotated raw:
```
VERSION: 03000000
FLAG: 0001
INPUTS: 01
0c571f65a11a383eb53896d93be77cccaa979234a7c93b1cbde63d82fde01b57 01000000 00 00000000 #I-0
OUTPUTS: 03 #0..2
e803000000000000 22 5120db7b995709b105a12719e1b4d92ab4bb616805fe0e2aee2db4b86efb
...
π¬ cedwies commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2307958813)
Was not yet able to reproduce the continuation. But just to clarify, @l0rinc : are we talking about a late-fail with βafterglowβ or does the node truly keep running?
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2307958813)
Was not yet able to reproduce the continuation. But just to clarify, @l0rinc : are we talking about a late-fail with βafterglowβ or does the node truly keep running?
π¬ stickies-v commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234213191)
Concept ACK. Price seems reasonable for the benefits, and we we're not locking ourselves in too much.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234213191)
Concept ACK. Price seems reasonable for the benefits, and we we're not locking ourselves in too much.
π¬ ajtowns commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3234222327)
> "33106" refers to the pull request with that number: #33106, which is also linked in the PR description (only 12 times). It's common practice to link to a related PR like this (e.g. for a followup) in the PR title.
Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3234222327)
> "33106" refers to the pull request with that number: #33106, which is also linked in the PR description (only 12 times). It's common practice to link to a related PR like this (e.g. for a followup) in the PR title.
Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.
π¬ ajtowns commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2307996095)
`-> Test -incrementalrelayfee=100.00000000sat/kvB...` doesn't seem very useful indeed.
mining_basic reports `-> Test -blockmintxfee=0.00000500 (500 sat/kvB)...` which seems strictly better, as it gives a value that can actually be used with the config param as well as a sats figure?
(https://github.com/bitcoin/bitcoin/pull/33189#discussion_r2307996095)
`-> Test -incrementalrelayfee=100.00000000sat/kvB...` doesn't seem very useful indeed.
mining_basic reports `-> Test -blockmintxfee=0.00000500 (500 sat/kvB)...` which seems strictly better, as it gives a value that can actually be used with the config param as well as a sats figure?
π¬ ajtowns commented on pull request "rpc: followups for 33106":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3234248084)
ACK daa40a3ff97346face9dcc64564010a66c91ccb2 ; cursory review, seems reasonable
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3234248084)
ACK daa40a3ff97346face9dcc64564010a66c91ccb2 ; cursory review, seems reasonable
π¬ jsarenik commented on issue "Zero output not cleared":
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3234271437)
From observation the important thing triggering the leftover (but spent, i.e. unusable) zero output inside a read-only descriptor wallet seem to be the two outputs to two different addresses in the final spend transaction. See https://mempool.space/signet/tx/af914f3f337bc84910d1d2d5d14556e5c2b07372ea99078d15c7a74c77c0b0e7?mode=details
Also see https://signet257.bublina.eu.org/sffrest.txt which may contain such transactions in the end of its output:
<img width="1080" height="1674" alt="Image" s
...
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3234271437)
From observation the important thing triggering the leftover (but spent, i.e. unusable) zero output inside a read-only descriptor wallet seem to be the two outputs to two different addresses in the final spend transaction. See https://mempool.space/signet/tx/af914f3f337bc84910d1d2d5d14556e5c2b07372ea99078d15c7a74c77c0b0e7?mode=details
Also see https://signet257.bublina.eu.org/sffrest.txt which may contain such transactions in the end of its output:
<img width="1080" height="1674" alt="Image" s
...
π¬ l0rinc commented on pull request "[IBD] coins: increase default UTXO flush batch size to 32 MiB":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3234329502)
Thanks a lot for testing this @hodlinator and @Eunovo.
It seems that different configurations behave differently here, so I've retested a few scenarios on a few different platforms to understand what the performance increase depends on exactly. It's not even a linear speedup (i.e. sometimes 32 MiB is faster, sometimes it's 64 MiB), some `dbcache` values reliably produce a lot faster results, while others reliably produce slower results for the same config (i.e. the before-after-plot for diffe
...
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3234329502)
Thanks a lot for testing this @hodlinator and @Eunovo.
It seems that different configurations behave differently here, so I've retested a few scenarios on a few different platforms to understand what the performance increase depends on exactly. It's not even a linear speedup (i.e. sometimes 32 MiB is faster, sometimes it's 64 MiB), some `dbcache` values reliably produce a lot faster results, while others reliably produce slower results for the same config (i.e. the before-after-plot for diffe
...
π¬ maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234350722)
Also, would be nice to have a pull request for https://github.com/bitcoin-core/qa-assets prepared. Possibly without caching is fine, if the msan-fuzz task fits in the GHA timeout.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3234350722)
Also, would be nice to have a pull request for https://github.com/bitcoin-core/qa-assets prepared. Possibly without caching is fine, if the msan-fuzz task fits in the GHA timeout.
π ajtowns's pull request is ready for review: "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)"
(https://github.com/bitcoin/bitcoin/pull/29843)
(https://github.com/bitcoin/bitcoin/pull/29843)
π¬ davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308112657)
In https://github.com/bitcoin/bitcoin/pull/32159/commits/57ce645f05d18d8ad10711c347a5989076f1f788 (_net: filter for default routes in netlink responses_)
Just info for other reviewers, I found this useful since I'm generally unfamiliar with how routing and gateways work:
As I understand, the destination prefix is a mask indicating what routes the gateway can be used for, a destination prefix of `0` (alternatively `0.0.0.0` or `::0`) indicates that the gateway can be used for all destinatio
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308112657)
In https://github.com/bitcoin/bitcoin/pull/32159/commits/57ce645f05d18d8ad10711c347a5989076f1f788 (_net: filter for default routes in netlink responses_)
Just info for other reviewers, I found this useful since I'm generally unfamiliar with how routing and gateways work:
As I understand, the destination prefix is a mask indicating what routes the gateway can be used for, a destination prefix of `0` (alternatively `0.0.0.0` or `::0`) indicates that the gateway can be used for all destinatio
...
π¬ l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2308173787)
I started it and accidentally left it in the background and it fetched 100k blocks - if it's an "afterglow", it's a resilient one. And I did reproduce it on 3 separate servers, seems like a real bug - though likely not introduced in this PR, will open an issue for it or investigate it myself later this week.
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2308173787)
I started it and accidentally left it in the background and it fetched 100k blocks - if it's an "afterglow", it's a resilient one. And I did reproduce it on 3 separate servers, seems like a real bug - though likely not introduced in this PR, will open an issue for it or investigate it myself later this week.
π¬ polespinasa commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3234490515)
> It is just dead code now, no?
@maflcko I think you're right.
Will squash after review, think it's easier this way.
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3234490515)
> It is just dead code now, no?
@maflcko I think you're right.
Will squash after review, think it's easier this way.
π¬ davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308207185)
In https://github.com/bitcoin/bitcoin/pull/32159/commits/42e99ad77396e4e9b02d67daf46349e215e99a0f (_net: skip non-route netlink responses_)
The `man 7 rtnetlink` page (https://man7.org/linux/man-pages/man7/rtnetlink.7.html) does not really offer much insight here, but as far as I can tell, the convention comes from the fact that the rtnetlink interface is also used for monitoring changes to the kernel's routing table, and because the userspace application is trying to maintain an idea of the
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308207185)
In https://github.com/bitcoin/bitcoin/pull/32159/commits/42e99ad77396e4e9b02d67daf46349e215e99a0f (_net: skip non-route netlink responses_)
The `man 7 rtnetlink` page (https://man7.org/linux/man-pages/man7/rtnetlink.7.html) does not really offer much insight here, but as far as I can tell, the convention comes from the fact that the rtnetlink interface is also used for monitoring changes to the kernel's routing table, and because the userspace application is trying to maintain an idea of the
...
π¬ polespinasa commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#discussion_r2308221628)
forgot to remove the line, will do next push
(https://github.com/bitcoin/bitcoin/pull/32138#discussion_r2308221628)
forgot to remove the line, will do next push
π fanquake merged a pull request: "ci: return to using dash in CentOS job"
(https://github.com/bitcoin/bitcoin/pull/33261)
(https://github.com/bitcoin/bitcoin/pull/33261)
π¬ polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308229142)
done
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308229142)
done
π¬ polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308230173)
I like to see `false` if it's not in b validation.
Fixed the "estimate of"
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308230173)
I like to see `false` if it's not in b validation.
Fixed the "estimate of"
π fanquake merged a pull request: "threading: reduce the scope of lock in getblocktemplate"
(https://github.com/bitcoin/bitcoin/pull/33264)
(https://github.com/bitcoin/bitcoin/pull/33264)
π fanquake merged a pull request: "rpc: followups for 33106"
(https://github.com/bitcoin/bitcoin/pull/33189)
(https://github.com/bitcoin/bitcoin/pull/33189)