💬 Sjors commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3388973256)
> I don't think this pull request changes anything about your workflow, if you connect to a different computer to compile and run.
I'm not worried about my own workflow for running a node, I was talking about us supporting it as a project. For that I need to compile it to occasionally test things, which is more work since I no longer have that machine on all day.
> The goal here is to drop support for an insecure base operating system
> (macOS Ventura 13.7.8 received its final security
...
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3388973256)
> I don't think this pull request changes anything about your workflow, if you connect to a different computer to compile and run.
I'm not worried about my own workflow for running a node, I was talking about us supporting it as a project. For that I need to compile it to occasionally test things, which is more work since I no longer have that machine on all day.
> The goal here is to drop support for an insecure base operating system
> (macOS Ventura 13.7.8 received its final security
...
💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2419042265)
Done in #33591
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2419042265)
Done in #33591
💬 fanquake commented on issue "Intermittent CI network issue downloading assets.json from GitHub":
(https://github.com/bitcoin/bitcoin/issues/33599#issuecomment-3389085662)
https://github.com/bitcoin/bitcoin/actions/runs/18386412219/job/52385904491?pr=28676
(https://github.com/bitcoin/bitcoin/issues/33599#issuecomment-3389085662)
https://github.com/bitcoin/bitcoin/actions/runs/18386412219/job/52385904491?pr=28676
💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3389106075)
> I'm not worried about my own workflow for running a node, I was talking about us supporting it as a project.
I think dropping support for an OS that hasn't received security updates for 6 months is fine. Also, you can still run a 30.x node until the release of 33.0 (https://bitcoincore.org/en/lifecycle/), which is 1.5 years. Also, you can compile and run any version yourself at your own risk (not using xcode).
Though, I am happy to review a pull request with the improvements you have in
...
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3389106075)
> I'm not worried about my own workflow for running a node, I was talking about us supporting it as a project.
I think dropping support for an OS that hasn't received security updates for 6 months is fine. Also, you can still run a 30.x node until the release of 33.0 (https://bitcoincore.org/en/lifecycle/), which is 1.5 years. Also, you can compile and run any version yourself at your own risk (not using xcode).
Though, I am happy to review a pull request with the improvements you have in
...
💬 fanquake commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3389122885)
#33518 has gone in, so this could be rebased / undrafted.
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3389122885)
#33518 has gone in, so this could be rebased / undrafted.
👋 fanquake's pull request is ready for review: "[29.x] Finalise 29.2"
(https://github.com/bitcoin/bitcoin/pull/33551)
(https://github.com/bitcoin/bitcoin/pull/33551)
💬 fanquake commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3389351214)
Release post for the website: https://github.com/bitcoin-core/bitcoincore.org/pull/1182.
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3389351214)
Release post for the website: https://github.com/bitcoin-core/bitcoincore.org/pull/1182.
💬 instagibbs commented on pull request "[29.x] Finalise 29.2":
(https://github.com/bitcoin/bitcoin/pull/33551#issuecomment-3389387280)
ACK 46d9b9091baa096da30da5e14329a32f1264229a
(https://github.com/bitcoin/bitcoin/pull/33551#issuecomment-3389387280)
ACK 46d9b9091baa096da30da5e14329a32f1264229a
💬 hebasto commented on pull request "cmake: Use builtin support for .manifest files":
(https://github.com/bitcoin/bitcoin/pull/33585#issuecomment-3389448715)
> > Is there any CMake's docs regarding handling `.manifest` files?
>
> https://cmake.org/cmake/help/latest/release/3.4.html#other
Thank you for the link!
The release notes mentioned above state:
> Manifest files ... will be merged with linker-generated manifests and embedded in the binary.
This PR enables linker-generated manifests by removing the `/MANIFEST:NO` linker flag. However, I still have a couple of questions:
1. What is the content of the linker-generated manifest? Wha
...
(https://github.com/bitcoin/bitcoin/pull/33585#issuecomment-3389448715)
> > Is there any CMake's docs regarding handling `.manifest` files?
>
> https://cmake.org/cmake/help/latest/release/3.4.html#other
Thank you for the link!
The release notes mentioned above state:
> Manifest files ... will be merged with linker-generated manifests and embedded in the binary.
This PR enables linker-generated manifests by removing the `/MANIFEST:NO` linker flag. However, I still have a couple of questions:
1. What is the content of the linker-generated manifest? Wha
...
💬 hebasto commented on issue "ci: remove third-party javascript usage from Windows CI":
(https://github.com/bitcoin/bitcoin/issues/32508#issuecomment-3389691137)
Maybe keep this issue open for visibility until the `ilammy/msvc-dev-cmd` action is actually removed from the repository whitelist?
This may, in turn, require backporting some commits from bitcoin/bitcoin#32513 and waiting until for contributors to rebase their PRs.
(https://github.com/bitcoin/bitcoin/issues/32508#issuecomment-3389691137)
Maybe keep this issue open for visibility until the `ilammy/msvc-dev-cmd` action is actually removed from the repository whitelist?
This may, in turn, require backporting some commits from bitcoin/bitcoin#32513 and waiting until for contributors to rebase their PRs.
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2419905179)
not going to nag any but until now in TxGraph we haven't been using conditionals to "save" us in release builds when these assumptions are violated
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2419905179)
not going to nag any but until now in TxGraph we haven't been using conditionals to "save" us in release builds when these assumptions are violated
💬 GrayHatGuy commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2419946561)
A proper risk analysis would have vetted this
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2419946561)
A proper risk analysis would have vetted this
💬 hebasto commented on pull request "DRAFT: add a freebsd job using systemlibs":
(https://github.com/bitcoin/bitcoin/pull/33562#issuecomment-3389904632)
> This will need permitting of the `vmactions/freebsd-vm@v1` action in this repo.
A few comments for historical context:
- https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128649877:
> Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
>
> No objection to merging this, but I'd say unless it
...
(https://github.com/bitcoin/bitcoin/pull/33562#issuecomment-3389904632)
> This will need permitting of the `vmactions/freebsd-vm@v1` action in this repo.
A few comments for historical context:
- https://github.com/bitcoin/bitcoin/pull/30164#issuecomment-2128649877:
> Not sure. What was the conclusion when GHA was added to the repo about enabling third party actions? IIRC it was unclear whether malicious or backdoored actions could compromise the repo, so the conclusion was to refrain from enabling them?
>
> No objection to merging this, but I'd say unless it
...
💬 purpleKarrot commented on pull request "cmake: Use builtin support for .manifest files":
(https://github.com/bitcoin/bitcoin/pull/33585#issuecomment-3389932816)
> What is the logic behind merging `${target}.manifest` with a linker-generated manifest?
The logic is internal to the MSVC linker. See: https://learn.microsoft.com/en-us/cpp/build/reference/manifestinput-specify-manifest-input
> 2. I'm not entirely sure that any fix of https://gitlab.kitware.com/cmake/cmake/-/issues/23244 will follow the same logic for MinGW as for MSVC.
We can never be entirely sure. But we can expect that different outcomes for MinGW as for MSVC should not be conside
...
(https://github.com/bitcoin/bitcoin/pull/33585#issuecomment-3389932816)
> What is the logic behind merging `${target}.manifest` with a linker-generated manifest?
The logic is internal to the MSVC linker. See: https://learn.microsoft.com/en-us/cpp/build/reference/manifestinput-specify-manifest-input
> 2. I'm not entirely sure that any fix of https://gitlab.kitware.com/cmake/cmake/-/issues/23244 will follow the same logic for MinGW as for MSVC.
We can never be entirely sure. But we can expect that different outcomes for MinGW as for MSVC should not be conside
...
💬 Crypt-iQ commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2420169460)
> I'm not sure what scenario you have in mind - a fork in which our active chain reorged away from the peer's best chain? This seems hypothetical enough that performance is probably not too important.
Is my understanding correct that `pindexLastCommonBlock` could be different across peers on the same chain as us (depending on which peers are processed first) or different because our peers may be on different chains from each other? I was just thinking out loud about the second case, where per
...
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2420169460)
> I'm not sure what scenario you have in mind - a fork in which our active chain reorged away from the peer's best chain? This seems hypothetical enough that performance is probably not too important.
Is my understanding correct that `pindexLastCommonBlock` could be different across peers on the same chain as us (depending on which peers are processed first) or different because our peers may be on different chains from each other? I was just thinking out loud about the second case, where per
...
👍 instagibbs approved a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3323336944)
reACK 7421e58dee402ee8b5b58a18684953a89ad350d7
`git range-diff master 5ac32aa441bf3ec1f1fa826f689405bc4a7b7354 7421e58dee402ee8b5b58a18684953a89ad350d7`
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3323336944)
reACK 7421e58dee402ee8b5b58a18684953a89ad350d7
`git range-diff master 5ac32aa441bf3ec1f1fa826f689405bc4a7b7354 7421e58dee402ee8b5b58a18684953a89ad350d7`
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2420073068)
de8ebdd795e35644435c7b68b2c2bc5f54ea9d7c
Not immediately clear this is necessary. These are all freshly generated, and shouldn't have holes, and capacities should be proportional?
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2420073068)
de8ebdd795e35644435c7b68b2c2bc5f54ea9d7c
Not immediately clear this is necessary. These are all freshly generated, and shouldn't have holes, and capacities should be proportional?
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2420320280)
No holes, indeed. But the capacities may be higher than needed, because transactions have been added one by one through `vector::push_back` in `m_mapping`, `m_linearization`, and `m_depgraph.entries`. When vector addition causes capacity to be exceeded, most C++ stdlibs use some exponential growing (e.g. doubling the capacity every time).
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2420320280)
No holes, indeed. But the capacities may be higher than needed, because transactions have been added one by one through `vector::push_back` in `m_mapping`, `m_linearization`, and `m_depgraph.entries`. When vector addition causes capacity to be exceeded, most C++ stdlibs use some exponential growing (e.g. doubling the capacity every time).
💬 jotapea commented on issue "Decouple datacarrier size and count limits (Draft PR)":
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3390264891)
@delta1 Let me help clarify anything you might need help understanding. Ai slop is generally useless. This is the opposite, a concise and straightforward upgrade to improve OP_RETURN relay policy configuration.
(https://github.com/bitcoin/bitcoin/issues/33595#issuecomment-3390264891)
@delta1 Let me help clarify anything you might need help understanding. Ai slop is generally useless. This is the opposite, a concise and straightforward upgrade to improve OP_RETURN relay policy configuration.
🤔 luke-jr requested changes to a pull request: "Add coins (UTXOs) tab and makes it view-only"
(https://github.com/bitcoin-core/gui/pull/898#pullrequestreview-3323706446)
Should probably only be available if advanced coin control is enabled in settings, and maybe only on a menu even then (if it made sense to do it as a tab, it should be within the main window, not a new window).
The SVG source for the new icon should also be included.
(https://github.com/bitcoin-core/gui/pull/898#pullrequestreview-3323706446)
Should probably only be available if advanced coin control is enabled in settings, and maybe only on a menu even then (if it made sense to do it as a tab, it should be within the main window, not a new window).
The SVG source for the new icon should also be included.