💬 instagibbs commented on pull request "test: add creating/spending validity checks for rare output scripts":
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1691351307)
not sure it's worth the effort for such a niche thing that already has unit test coverage
(https://github.com/bitcoin/bitcoin/pull/30481#discussion_r1691351307)
not sure it's worth the effort for such a niche thing that already has unit test coverage
💬 hebasto commented on pull request "guix: GCC 12 consolidation":
(https://github.com/bitcoin/bitcoin/pull/30511#issuecomment-2250206015)
My Guix build:
```
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu.tar.gz
1db81362865b56197
...
(https://github.com/bitcoin/bitcoin/pull/30511#issuecomment-2250206015)
My Guix build:
```
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu.tar.gz
1db81362865b56197
...
📝 maflcko opened a pull request: "doc: rpc: Use "output script" consistently (2/2)"
(https://github.com/bitcoin/bitcoin/pull/30524)
Small follow-up to https://github.com/bitcoin/bitcoin/pull/30408 to fixup the three RPCs that were forgotten.
(https://github.com/bitcoin/bitcoin/pull/30524)
Small follow-up to https://github.com/bitcoin/bitcoin/pull/30408 to fixup the three RPCs that were forgotten.
💬 maflcko commented on pull request "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results":
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2250209869)
> Affects the help text of the following RPCs:
Looks like 3 RPCs were forgotten, so I created https://github.com/bitcoin/bitcoin/pull/30524
(https://github.com/bitcoin/bitcoin/pull/30408#issuecomment-2250209869)
> Affects the help text of the following RPCs:
Looks like 3 RPCs were forgotten, so I created https://github.com/bitcoin/bitcoin/pull/30524
💬 maflcko commented on pull request "rpc: Return errors in loadtxoutset that currently go to logs":
(https://github.com/bitcoin/bitcoin/pull/30497#issuecomment-2250213872)
Rebased. Should be trivial to re-ACK with `git range-diff bitcoin-core/master fa5c253b4e fa91404c68`
(https://github.com/bitcoin/bitcoin/pull/30497#issuecomment-2250213872)
Rebased. Should be trivial to re-ACK with `git range-diff bitcoin-core/master fa5c253b4e fa91404c68`
💬 willcl-ark commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1691365458)
There's some duplication in the first three sentences (repeating that we bind to 127.0.0.1:8334) that can perhaps be removed.
Additionally, not mentioned is the new startup behaviour where we abort on failure to bind to any P2P port.
Suggestion:
```suggestion
- Previously if Bitcoin Core was listening for P2P connections, either using
default settings or via `bind=addr:port` it would always also bind to
`127.0.0.1:8334` to listen for Tor connections. It was not possible to switch
...
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1691365458)
There's some duplication in the first three sentences (repeating that we bind to 127.0.0.1:8334) that can perhaps be removed.
Additionally, not mentioned is the new startup behaviour where we abort on failure to bind to any P2P port.
Suggestion:
```suggestion
- Previously if Bitcoin Core was listening for P2P connections, either using
default settings or via `bind=addr:port` it would always also bind to
`127.0.0.1:8334` to listen for Tor connections. It was not possible to switch
...
👍 hebasto approved a pull request: "guix: GCC 12 consolidation"
(https://github.com/bitcoin/bitcoin/pull/30511#pullrequestreview-2199185214)
ACK d1592d2eee1913e734a4f92907e796eb3350c64a.
(https://github.com/bitcoin/bitcoin/pull/30511#pullrequestreview-2199185214)
ACK d1592d2eee1913e734a4f92907e796eb3350c64a.
👍 fanquake approved a pull request: "depends: Cleanup postprocess commands after switching to CMake"
(https://github.com/bitcoin/bitcoin/pull/30506#pullrequestreview-2199187950)
ACK a0314c151679a348d842b68c5ecb7a556700811c
Guix build (aarch64):
```bash
a35aa8777223dbb7fdb3b9f600067303a5846c26a854d7d5aaa41b5fe12a047f guix-build-a0314c151679/output/aarch64-linux-gnu/SHA256SUMS.part
917c0ebbb4c8cbddc6e314a2f6bcee22d9399356b56ecdecd6fb008882a6eb48 guix-build-a0314c151679/output/aarch64-linux-gnu/bitcoin-a0314c151679-aarch64-linux-gnu-debug.tar.gz
884d1264b2d8bcb2975c61248f5794fba26c60deb445a07f12b69c17db8c4cc6 guix-build-a0314c151679/output/aarch64-linux-gnu/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/30506#pullrequestreview-2199187950)
ACK a0314c151679a348d842b68c5ecb7a556700811c
Guix build (aarch64):
```bash
a35aa8777223dbb7fdb3b9f600067303a5846c26a854d7d5aaa41b5fe12a047f guix-build-a0314c151679/output/aarch64-linux-gnu/SHA256SUMS.part
917c0ebbb4c8cbddc6e314a2f6bcee22d9399356b56ecdecd6fb008882a6eb48 guix-build-a0314c151679/output/aarch64-linux-gnu/bitcoin-a0314c151679-aarch64-linux-gnu-debug.tar.gz
884d1264b2d8bcb2975c61248f5794fba26c60deb445a07f12b69c17db8c4cc6 guix-build-a0314c151679/output/aarch64-linux-gnu/bitcoin
...
💬 fanquake commented on pull request "depends: remove Darwin ENV unsetting":
(https://github.com/bitcoin/bitcoin/pull/30451#discussion_r1691383416)
Heh, yea. These should/would only be needed for macOS, and only for Qt-related code. I guess everything still currently builds even with them unset, because when we compile our objc++ code, we would never be looking in any GCC related include/std-lib/related dirs in any case? Will push some changes.
(https://github.com/bitcoin/bitcoin/pull/30451#discussion_r1691383416)
Heh, yea. These should/would only be needed for macOS, and only for Qt-related code. I guess everything still currently builds even with them unset, because when we compile our objc++ code, we would never be looking in any GCC related include/std-lib/related dirs in any case? Will push some changes.
🚀 fanquake merged a pull request: "rest: Reject truncated hex txid early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30482)
(https://github.com/bitcoin/bitcoin/pull/30482)
🚀 fanquake merged a pull request: "test: Add arguments for creating a slimmer TestingSetup"
(https://github.com/bitcoin/bitcoin/pull/30399)
(https://github.com/bitcoin/bitcoin/pull/30399)
🚀 fanquake merged a pull request: "guix: GCC 12 consolidation"
(https://github.com/bitcoin/bitcoin/pull/30511)
(https://github.com/bitcoin/bitcoin/pull/30511)
🚀 fanquake merged a pull request: "depends: Cleanup postprocess commands after switching to CMake"
(https://github.com/bitcoin/bitcoin/pull/30506)
(https://github.com/bitcoin/bitcoin/pull/30506)
👍 theStack approved a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2199234365)
re-ACK 7c29e556c573a63351096c34bc072ae0c60ffa29
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2199234365)
re-ACK 7c29e556c573a63351096c34bc072ae0c60ffa29
👍 dergoegge approved a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2199244448)
reACK 7c29e556c573a63351096c34bc072ae0c60ffa29
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2199244448)
reACK 7c29e556c573a63351096c34bc072ae0c60ffa29
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2250281391)
reACK 448374f2c90 via range-diff
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2250281391)
reACK 448374f2c90 via range-diff
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691416836)
Sorry, no, the original code was right, and there is more mixed up in the diagrams. Sorry...
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691416836)
Sorry, no, the original code was right, and there is more mixed up in the diagrams. Sorry...
🚀 fanquake merged a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507)
(https://github.com/bitcoin/bitcoin/pull/30507)
💬 glozow commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691423677)
Are we meant to end up with a child, not a parent? And the pairs are supposed to be cpfps?
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691423677)
Are we meant to end up with a child, not a parent? And the pairs are supposed to be cpfps?
💬 ryanofsky commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2250297907)
> Seems like if a script such as this is going to exist, it should be getting run as part of the CI, so that regressions are actually caught (if that's what we want to do)?
I think it would be nice to run as part of CI. I added a commit to try that.
If we think the script is a maintenance burden, it is also not essential to keep. I wrote it mainly to help write #29015 to figure out which parts of the util library should be moved to common, crypto, and consensus libraries.
> note: `libbi
...
(https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2250297907)
> Seems like if a script such as this is going to exist, it should be getting run as part of the CI, so that regressions are actually caught (if that's what we want to do)?
I think it would be nice to run as part of CI. I added a commit to try that.
If we think the script is a maintenance burden, it is also not essential to keep. I wrote it mainly to help write #29015 to figure out which parts of the util library should be moved to common, crypto, and consensus libraries.
> note: `libbi
...
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691443552)
I've updated the diagrams higher up in the thread to match the actual code (both had several errors...). Let me verify whether they also actually have the complexity properties I claim they have, and then I'll do a push to fix it. Sorry for the mess.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1691443552)
I've updated the diagrams higher up in the thread to match the actual code (both had several errors...). Let me verify whether they also actually have the complexity properties I claim they have, and then I'll do a push to fix it. Sorry for the mess.