🤔 glozow reviewed a pull request: "feefrac: avoid explicitly computing diagram; compare based on chunks"
(https://github.com/bitcoin/bitcoin/pull/29757#pullrequestreview-1983373180)
reACK c2fdcf4
(https://github.com/bitcoin/bitcoin/pull/29757#pullrequestreview-1983373180)
reACK c2fdcf4
💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553766100)
mm yeah let me make this a bit more bullet proof
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553766100)
mm yeah let me make this a bit more bullet proof
💬 Sjors commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2039962940)
You can't know which configuration @sipa - or anyone else running that code - is using. And keeping track of all permutations gets quite noisy.
For my own seed I've occasionally added more filters for new features that were being developed. I wouldn't want to keep making pull requests to change the comment (mine doesn't mention any).
Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2039962940)
You can't know which configuration @sipa - or anyone else running that code - is using. And keeping track of all permutations gets quite noisy.
For my own seed I've occasionally added more filters for new features that were being developed. I wouldn't want to keep making pull requests to change the comment (mine doesn't mention any).
Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py
💬 hebasto commented on pull request "ci: Drop duplicated compiler flags":
(https://github.com/bitcoin/bitcoin/pull/29800#discussion_r1553772500)
The `--with-sanitizers=memory` has been restored in the recent push.
(https://github.com/bitcoin/bitcoin/pull/29800#discussion_r1553772500)
The `--with-sanitizers=memory` has been restored in the recent push.
💬 glozow commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553777536)
Well I don't think you need get rid of the transactions here - you just want to make sure they get cleared by the end of the test right?
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1553777536)
Well I don't think you need get rid of the transactions here - you just want to make sure they get cleared by the end of the test right?
💬 fanquake commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2039984016)
Pulled this into 27.x.
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2039984016)
Pulled this into 27.x.
💬 TheCharlatan commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039994022)
I am building depends on macos 12.7.4 and am not seeing any patch related errors or warning. Seems weird that this suddenly pops up now after having worked for a long time? I checked my `brew list` and don't have any entries for "patch" or "gpatch" and when I run `which patch`, it is in /usr/bin, so seems like it is just the default program?
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039994022)
I am building depends on macos 12.7.4 and am not seeing any patch related errors or warning. Seems weird that this suddenly pops up now after having worked for a long time? I checked my `brew list` and don't have any entries for "patch" or "gpatch" and when I run `which patch`, it is in /usr/bin, so seems like it is just the default program?
💬 fanquake commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039996663)
Yea. I'm also sure that my macOS 13.6.6 system is using the default apple patch (will double check), and I don't see any issues.
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039996663)
Yea. I'm also sure that my macOS 13.6.6 system is using the default apple patch (will double check), and I don't see any issues.
💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2040012852)
Rebased after #29419.
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2040012852)
Rebased after #29419.
💬 maflcko commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040023180)
Without knowing the cause, there is little that can be done. Given that WSL-built binaries didn't regress, it hints at some build flags inside of guix. My suggestion would be to bisect while doing guix builds. An alternative would be to try to match the guix compile flags and compiler version in WSL.
Also, instead of IBD, the benchmarks can be used, if they differ enough. Though, they'll need to be enabled:
https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034314366
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040023180)
Without knowing the cause, there is little that can be done. Given that WSL-built binaries didn't regress, it hints at some build flags inside of guix. My suggestion would be to bisect while doing guix builds. An alternative would be to try to match the guix compile flags and compiler version in WSL.
Also, instead of IBD, the benchmarks can be used, if they differ enough. Though, they'll need to be enabled:
https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034314366
💬 maflcko commented on pull request "ci: Drop duplicated compiler flags":
(https://github.com/bitcoin/bitcoin/pull/29800#issuecomment-2040027759)
re-ACK a3485af67da4949c72c45acc608f8746ed0e0848
(https://github.com/bitcoin/bitcoin/pull/29800#issuecomment-2040027759)
re-ACK a3485af67da4949c72c45acc608f8746ed0e0848
💬 fjahr commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#discussion_r1553824794)
That framing makes a lot more sense to me 👍
(https://github.com/bitcoin/bitcoin/pull/29690#discussion_r1553824794)
That framing makes a lot more sense to me 👍
💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040045243)
@maflcko Just to clarify, what didn't regress was the release binary for Linux (which I assume is built with guix, not WSL) running in WSL.
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040045243)
@maflcko Just to clarify, what didn't regress was the release binary for Linux (which I assume is built with guix, not WSL) running in WSL.
💬 fanquake commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040086861)
> which I assume is built with guix
All of the release binaries are built using Guix. Windows is produced in Guix using GCC+Mingw-w64. We don't produce any release binaries using WSL or MSVC.
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040086861)
> which I assume is built with guix
All of the release binaries are built using Guix. Windows is produced in Guix using GCC+Mingw-w64. We don't produce any release binaries using WSL or MSVC.
💬 fanquake commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2040102948)
Concept ACK. I think tracking where any recursion currently is, is useful, as well as being alerted to new usage in the future.
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2040102948)
Concept ACK. I think tracking where any recursion currently is, is useful, as well as being alerted to new usage in the future.
👍 fanquake approved a pull request: "refactor: Remove gmtime*"
(https://github.com/bitcoin/bitcoin/pull/29081#pullrequestreview-1983608799)
ACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f - more std lib & even less stuff to port.
(https://github.com/bitcoin/bitcoin/pull/29081#pullrequestreview-1983608799)
ACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f - more std lib & even less stuff to port.
🚀 fanquake merged a pull request: "refactor: Remove gmtime*"
(https://github.com/bitcoin/bitcoin/pull/29081)
(https://github.com/bitcoin/bitcoin/pull/29081)
💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#issuecomment-2040171293)
> Any chance to turn the first commit into an scripted-diff?
The commit message was misleading It's not just renaming `max_weight`, it also updates the input parameter docstring to match match the new name.
And also the fuzz test renames has more info like `low_max_selection_weight` and `high_max_selection_weight`.
I've updated the commit message of the first commit https://github.com/bitcoin/bitcoin/pull/29523/commits/270639324a6637e0025e1458221ab488a8a09554
(https://github.com/bitcoin/bitcoin/pull/29523#issuecomment-2040171293)
> Any chance to turn the first commit into an scripted-diff?
The commit message was misleading It's not just renaming `max_weight`, it also updates the input parameter docstring to match match the new name.
And also the fuzz test renames has more info like `low_max_selection_weight` and `high_max_selection_weight`.
I've updated the commit message of the first commit https://github.com/bitcoin/bitcoin/pull/29523/commits/270639324a6637e0025e1458221ab488a8a09554
👍 fanquake approved a pull request: "ci: Drop duplicated compiler flags"
(https://github.com/bitcoin/bitcoin/pull/29800#pullrequestreview-1983625214)
ACK a3485af67da4949c72c45acc608f8746ed0e0848 - no-longer a change in behaviour.
(https://github.com/bitcoin/bitcoin/pull/29800#pullrequestreview-1983625214)
ACK a3485af67da4949c72c45acc608f8746ed0e0848 - no-longer a change in behaviour.
💬 fanquake commented on pull request "[WIP] ci: test secp256k1 MSAN asm annotations":
(https://github.com/bitcoin/bitcoin/pull/29742#discussion_r1553941342)
Will be done via #29800.
(https://github.com/bitcoin/bitcoin/pull/29742#discussion_r1553941342)
Will be done via #29800.