💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380554457)
the full challenge should be copy and pasted - please test
cc: @luke-jr @pablomartin4btc
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380554457)
the full challenge should be copy and pasted - please test
cc: @luke-jr @pablomartin4btc
💬 polespinasa commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-3336297423)
Rebased on top of master (after #33230 was merged).
The diff is simple:
```diff
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 320823b1c1..acbdc14db8 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -37,7 +37,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "generatetoaddress", 2, "maxtries" },
{ "generatetodescriptor", 0, "num_blocks" },
{ "generatetodescriptor", 2, "maxtries" },
- //{ "generateblock", 0, "outputs"},
+ { "gener
...
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-3336297423)
Rebased on top of master (after #33230 was merged).
The diff is simple:
```diff
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 320823b1c1..acbdc14db8 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -37,7 +37,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "generatetoaddress", 2, "maxtries" },
{ "generatetodescriptor", 0, "num_blocks" },
{ "generatetodescriptor", 2, "maxtries" },
- //{ "generateblock", 0, "outputs"},
+ { "gener
...
💬 151henry151 commented on pull request "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3336328151)
> Please amend the comments in `contrib/guix/libexec/build.sh` as noted above.
I've made the recommended amendments, thanks for the guidance.
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3336328151)
> Please amend the comments in `contrib/guix/libexec/build.sh` as noted above.
I've made the recommended amendments, thanks for the guidance.
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380613802)
I can't recall if this was discussed elsewhere, but in some cases when the path ends in a relative specifier the parent directory name can't be recovered without filesystem access: e.g. the wallet name is `../`, I think filesystem access might be necessary here.
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380613802)
I can't recall if this was discussed elsewhere, but in some cases when the path ends in a relative specifier the parent directory name can't be recovered without filesystem access: e.g. the wallet name is `../`, I think filesystem access might be necessary here.
💬 ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380631535)
It was discussed, see https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-2983599593:
> I don't think this is true. If a wallet ascends more than it descends like `../../../../mywallet`, the suggested change would use "mywallet" as the prefix. If the path contains `.` components or internal `..`, `lexically_normal` will strip them out. If the path contains trailing slashes, the suggested code strips them out to. If the path is literally just `..` or `../..` or similar even that is
...
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380631535)
It was discussed, see https://github.com/bitcoin/bitcoin/pull/32273#pullrequestreview-2983599593:
> I don't think this is true. If a wallet ascends more than it descends like `../../../../mywallet`, the suggested change would use "mywallet" as the prefix. If the path contains `.` components or internal `..`, `lexically_normal` will strip them out. If the path contains trailing slashes, the suggested code strips them out to. If the path is literally just `..` or `../..` or similar even that is
...
💬 davidgumberg commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380642390)
I agree, maybe I had too strict of an idea of what the expectation would be for the destination of the backup: if the only goal is that the backup filename be safe and likely to work, then `lexically_normal()` is fine, and satisfying those requirements is probably good enough for the migration backup, but I'm still not sure I understand what is wrong with using the filesystem here.
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2380642390)
I agree, maybe I had too strict of an idea of what the expectation would be for the destination of the backup: if the only goal is that the backup filename be safe and likely to work, then `lexically_normal()` is fine, and satisfying those requirements is probably good enough for the migration backup, but I'm still not sure I understand what is wrong with using the filesystem here.
💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2380772877)
I have pushed a new version (rebased, extended test), let me know what you think.
I have implemented a very simple multithreaded version but I couldn't convince it to achieve *any* speedup whatsoever - I guess xor operations are a lot cheaper than disk reads/writes. The total CPU usage was at 20% even with 50 threads.
I have pushed my threaded solution to [l0rinc/bitcoin@`fb75395` (#40)](https://github.com/l0rinc/bitcoin/pull/40/commits/fb75395c2164c043b8696a43b9d116d953f90800#diff-b1e1919
...
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2380772877)
I have pushed a new version (rebased, extended test), let me know what you think.
I have implemented a very simple multithreaded version but I couldn't convince it to achieve *any* speedup whatsoever - I guess xor operations are a lot cheaper than disk reads/writes. The total CPU usage was at 20% even with 50 threads.
I have pushed my threaded solution to [l0rinc/bitcoin@`fb75395` (#40)](https://github.com/l0rinc/bitcoin/pull/40/commits/fb75395c2164c043b8696a43b9d116d953f90800#diff-b1e1919
...
✅ amishhaa closed a pull request: "contrib: fix for macOS deployment build failing on Qt translations even though it is optional."
(https://github.com/bitcoin/bitcoin/pull/33358)
(https://github.com/bitcoin/bitcoin/pull/33358)
📝 amishhaa opened a pull request: "contrib: fix for macOS deployment build failing on Qt translations even though it is optional."
(https://github.com/bitcoin/bitcoin/pull/33482)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Qt
...
(https://github.com/bitcoin/bitcoin/pull/33482)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Qt
...
💬 amishhaa commented on pull request "contrib: fix for macOS deployment build failing on Qt translations even though it is optional.":
(https://github.com/bitcoin/bitcoin/pull/33482#issuecomment-3336844540)
Refer PR: #33358 for previous discussions.
(https://github.com/bitcoin/bitcoin/pull/33482#issuecomment-3336844540)
Refer PR: #33358 for previous discussions.
💬 amishhaa commented on pull request "contrib: fix for macOS deployment build failing on Qt translations even though it is optional.":
(https://github.com/bitcoin/bitcoin/pull/33358#issuecomment-3336848208)
Closed this PR due to some local branching issues. Reopened with a clean working tree. (cc: @hebasto, @fanquake )
(https://github.com/bitcoin/bitcoin/pull/33358#issuecomment-3336848208)
Closed this PR due to some local branching issues. Reopened with a clean working tree. (cc: @hebasto, @fanquake )
🤔 hebasto reviewed a pull request: "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270544945)
My Guix build:
```
aarch64
ab375345e39248d2c61fe6fce1afeb7b95623cfc9db452a7cfbd5b6797bf9c16 guix-build-d55753626292/output/aarch64-linux-gnu/SHA256SUMS.part
85bd20c1b7076387d8efcb5ade58d29774f8114a712492a77315627d0db41094 guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu-debug.tar.gz
f52e90c92d99b17c916b2c3ff2b57608597ca39ec7324ff5840550636cfd3a50 guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu.tar.gz
9c8c935f
...
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270544945)
My Guix build:
```
aarch64
ab375345e39248d2c61fe6fce1afeb7b95623cfc9db452a7cfbd5b6797bf9c16 guix-build-d55753626292/output/aarch64-linux-gnu/SHA256SUMS.part
85bd20c1b7076387d8efcb5ade58d29774f8114a712492a77315627d0db41094 guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu-debug.tar.gz
f52e90c92d99b17c916b2c3ff2b57608597ca39ec7324ff5840550636cfd3a50 guix-build-d55753626292/output/aarch64-linux-gnu/bitcoin-d55753626292-aarch64-linux-gnu.tar.gz
9c8c935f
...
💬 maflcko commented on pull request "test/refactor: use test deque to avoid quadratic iteration":
(https://github.com/bitcoin/bitcoin/pull/33313#issuecomment-3337139398)
lgtm ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1
This shouldn't affect performance, but it seems fine as a style-cleanup/refactor.
(https://github.com/bitcoin/bitcoin/pull/33313#issuecomment-3337139398)
lgtm ACK 75e6984ec8c6fa196ad78c11f454da506d7c8ff1
This shouldn't affect performance, but it seems fine as a style-cleanup/refactor.
👍 hebasto approved a pull request: "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270575407)
ACK d55753626292be17fbb300b54601942ff2730729.
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270575407)
ACK d55753626292be17fbb300b54601942ff2730729.
🤔 w0xlt reviewed a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3270590403)
reACK https://github.com/bitcoin/bitcoin/pull/33313/commits/75e6984ec8c6fa196ad78c11f454da506d7c8ff1
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3270590403)
reACK https://github.com/bitcoin/bitcoin/pull/33313/commits/75e6984ec8c6fa196ad78c11f454da506d7c8ff1
💬 0xB10C commented on issue "Ability to retrieve peer info by peer id":
(https://github.com/bitcoin/bitcoin/issues/33478#issuecomment-3337172425)
for reference, this seems related to https://github.com/bitcoin/bitcoin/pull/32741
(https://github.com/bitcoin/bitcoin/issues/33478#issuecomment-3337172425)
for reference, this seems related to https://github.com/bitcoin/bitcoin/pull/32741
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2381229968)
Looks like you are suggesting we do something more invasive. Can you clarify what you mean?
I do not believe something more invasive is required since this simple fix works, but I'm happy to hear what others think.
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2381229968)
Looks like you are suggesting we do something more invasive. Can you clarify what you mean?
I do not believe something more invasive is required since this simple fix works, but I'm happy to hear what others think.
🤔 janb84 reviewed a pull request: "build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270732296)
re ACK d55753626292be17fbb300b54601942ff2730729
change since last ack:
- changes in comments in script.
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3270732296)
re ACK d55753626292be17fbb300b54601942ff2730729
change since last ack:
- changes in comments in script.
💬 maflcko commented on pull request "cmake: exclude secp256k1 from all":
(https://github.com/bitcoin/bitcoin/pull/33390#issuecomment-3337304020)
Could turn into draft while CI is red?
(https://github.com/bitcoin/bitcoin/pull/33390#issuecomment-3337304020)
Could turn into draft while CI is red?
💬 jmoik commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#discussion_r2381326351)
Good point, I was not aware of this variable, I have adjusted the code and tested it.
(https://github.com/bitcoin/bitcoin/pull/33297#discussion_r2381326351)
Good point, I was not aware of this variable, I have adjusted the code and tested it.
💬 maflcko commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#discussion_r2381335649)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
(https://github.com/bitcoin/bitcoin/pull/33297#discussion_r2381335649)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits