💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2876746382)
Friendly ping @laanwj @sipsorcery :)
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2876746382)
Friendly ping @laanwj @sipsorcery :)
🤔 maflcko reviewed a pull request: "doc: update unix build doc with build flags"
(https://github.com/bitcoin/bitcoin/pull/32269#pullrequestreview-2837037436)
lgtm ACK be6e4c4db80030aa100cadb68706601eae13c010
(https://github.com/bitcoin/bitcoin/pull/32269#pullrequestreview-2837037436)
lgtm ACK be6e4c4db80030aa100cadb68706601eae13c010
💬 maflcko commented on pull request "doc: update unix build doc with build flags":
(https://github.com/bitcoin/bitcoin/pull/32269#discussion_r2086979578)
nit: Not sure we want to manually number the sections. The order should already be clear
(https://github.com/bitcoin/bitcoin/pull/32269#discussion_r2086979578)
nit: Not sure we want to manually number the sections. The order should already be clear
💬 theStack commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876795632)
@edilmedeiros: I agree it's fine as-is for now and can easily extended later. Note though that including OP_2...OP_16 is trivial, as it's just an extended range to OP_1/OP_TRUE, i.e. a diff like this would likely be sufficient:
```diff
diff --git a/contrib/signet/miner b/contrib/signet/miner
index 27f7afba8d..b79fa5d4f8 100755
--- a/contrib/signet/miner
+++ b/contrib/signet/miner
@@ -245,8 +245,8 @@ def trivial_challenge(spkhex):
challenges such as OP_TRUE
"""
spk = bytes
...
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876795632)
@edilmedeiros: I agree it's fine as-is for now and can easily extended later. Note though that including OP_2...OP_16 is trivial, as it's just an extended range to OP_1/OP_TRUE, i.e. a diff like this would likely be sufficient:
```diff
diff --git a/contrib/signet/miner b/contrib/signet/miner
index 27f7afba8d..b79fa5d4f8 100755
--- a/contrib/signet/miner
+++ b/contrib/signet/miner
@@ -245,8 +245,8 @@ def trivial_challenge(spkhex):
challenges such as OP_TRUE
"""
spk = bytes
...
💬 maflcko commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087011545)
Seems fine, but it may be better to remove the suppression in the exact commit that removes the need for it. This would make it easier to revert the commit atomically in one go, if there is need for whatever reason.
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087011545)
Seems fine, but it may be better to remove the suppression in the exact commit that removes the need for it. This would make it easier to revert the commit atomically in one go, if there is need for whatever reason.
📝 pablomartin4btc opened a pull request: "wallet, refactor: Remove Legacy wallet unused warnings and errors"
(https://github.com/bitcoin/bitcoin/pull/32481)
Remove dead code due to legacy wallet support removal.
(https://github.com/bitcoin/bitcoin/pull/32481)
Remove dead code due to legacy wallet support removal.
💬 michaelfolkson commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876824169)
Concept ACK for this and Concept ACK for #32359 unless @luke-jr/Knots argues convincingly that the removal of these config options in Core makes maintaining them in Knots an excessive amount of work.
The objectives of Core's mempool policy and Knots mempool policy are totally different. Knots is trying to filter what it defines as spam and Core[ isn't](https://btctranscripts.com/adopting-bitcoin/2021/2021-11-16-gloria-zhao-transaction-relay-policy). I don't know if it makes sense for Core to
...
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876824169)
Concept ACK for this and Concept ACK for #32359 unless @luke-jr/Knots argues convincingly that the removal of these config options in Core makes maintaining them in Knots an excessive amount of work.
The objectives of Core's mempool policy and Knots mempool policy are totally different. Knots is trying to filter what it defines as spam and Core[ isn't](https://btctranscripts.com/adopting-bitcoin/2021/2021-11-16-gloria-zhao-transaction-relay-policy). I don't know if it makes sense for Core to
...
💬 fanquake commented on pull request "wallet, refactor: Remove Legacy wallet unused warnings and errors":
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2087016856)
You can drop the changes in here, this file will be updated in a translations update. @hebasto might be worth doing one shortly just to cleanup these strings, since they keep being removed in various PRs.
(https://github.com/bitcoin/bitcoin/pull/32481#discussion_r2087016856)
You can drop the changes in here, this file will be updated in a translations update. @hebasto might be worth doing one shortly just to cleanup these strings, since they keep being removed in various PRs.
💬 hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2876906638)
My Guix build:
```
aarch64
24986a84cc15356c2761a8d05898d33d60ff10431eecdf68d9fb758432a87f79 guix-build-915c1fa72c07/output/aarch64-linux-gnu/SHA256SUMS.part
be7bba66e8e84ba8428aed395a296e397d07f5d961bb8ab80bdd19d2c48589ba guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu-debug.tar.gz
a77553ea1b9eca35f6b9bc00f0be22f3016b8fdf1425b71ec93a6fdbcd8e154c guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu.tar.gz
4f1a82c8
...
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2876906638)
My Guix build:
```
aarch64
24986a84cc15356c2761a8d05898d33d60ff10431eecdf68d9fb758432a87f79 guix-build-915c1fa72c07/output/aarch64-linux-gnu/SHA256SUMS.part
be7bba66e8e84ba8428aed395a296e397d07f5d961bb8ab80bdd19d2c48589ba guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu-debug.tar.gz
a77553ea1b9eca35f6b9bc00f0be22f3016b8fdf1425b71ec93a6fdbcd8e154c guix-build-915c1fa72c07/output/aarch64-linux-gnu/bitcoin-915c1fa72c07-aarch64-linux-gnu.tar.gz
4f1a82c8
...
💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087061842)
Dropped and added `OP_2` as an example for inspiration.
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087061842)
Dropped and added `OP_2` as an example for inspiration.
💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087066202)
I switched to using a range, where it makes more sense to use a code comment.
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r2087066202)
I switched to using a range, where it makes more sense to use a code comment.
💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087066691)
Any accounting inconsistency could've caused it to become negative and fail, so instead of removing these suppressions in the last commit that fixed the problem (giving the impression that it was just one change that enabled us removing it), I did it separately instead.
Is this important, is this holding back the ACK from your part?
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2087066691)
Any accounting inconsistency could've caused it to become negative and fail, so instead of removing these suppressions in the last commit that fixed the problem (giving the impression that it was just one change that enabled us removing it), I did it separately instead.
Is this important, is this holding back the ACK from your part?
💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927264)
I agree with you, but I also prefer the current code; seems easier to understand.
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927264)
I agree with you, but I also prefer the current code; seems easier to understand.
💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927696)
I added `OP_2` through `OP_16`, with a new test case for the latter. It's not practical to catch _all_ possible trivial scripts though.
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876927696)
I added `OP_2` through `OP_16`, with a new test case for the latter. It's not practical to catch _all_ possible trivial scripts though.
📝 fanquake opened a pull request: "build: add `-W[leading|trailing]-whitespace`"
(https://github.com/bitcoin/bitcoin/pull/32482)
GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options. Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
```bash
[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing
...
(https://github.com/bitcoin/bitcoin/pull/32482)
GCC 15 now has options to turn these into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable these options. Unfortunately, CMake/Qt will generate code that contains trailing whitespace:
```bash
[ 98%] Building CXX object src/qt/CMakeFiles/bitcoinqt.dir/bitcoinqt_autogen/EJRQKI7XPS/qrc_bitcoin_locale.cpp.o
/root/bitcoin/build/src/qt/bitcoinqt_autogen/EWIEGA46WW/qrc_bitcoin.cpp:5603:2: warning: trailing
...
💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876932472)
Code Review ACK 6adec89e3961508875e79240a19bc981e5addf1a (untested)
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876932472)
Code Review ACK 6adec89e3961508875e79240a19bc981e5addf1a (untested)
💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087077335)
Did a git clean -fxd before running it so I think it should've covered only the tracked files.
------
Will let you decide of course which ones makes sense to include, just pointing out that we may want to extend this list.
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087077335)
Did a git clean -fxd before running it so I think it should've covered only the tracked files.
------
Will let you decide of course which ones makes sense to include, just pointing out that we may want to extend this list.
💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876941545)
Code Review reACK 2be93f7ba3866892d98795384779ce2e965753a1 (untested)
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876941545)
Code Review reACK 2be93f7ba3866892d98795384779ce2e965753a1 (untested)
💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087083142)
I haven't tested it, if you say it's fast enough, I believe you
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2087083142)
I haven't tested it, if you say it's fast enough, I believe you
💬 l0rinc commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#issuecomment-2876944750)
ACK fa9198af55df74b0c19c9125d256ad4df83cf005
(https://github.com/bitcoin/bitcoin/pull/32477#issuecomment-2876944750)
ACK fa9198af55df74b0c19c9125d256ad4df83cf005
🤔 caesrcd reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2837204950)
reACK e98c51fcce
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2837204950)
reACK e98c51fcce