💬 GBKS commented on pull request "wallet: Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-2124671596)
Good points. I think it could work well to combine the two warnings to a single dialog. It could be placed between the initial `Create wallet` dialog, and the password input.
1. **Create wallet dialog:** The user checks `Encrypt wallet` and clicks `Continue`.
2. **Password warning dialog:** The application informs the user about the risks of using a password and offers `Back` and `Continue` options. The user considers, decides to move forward with the password, and clicks `Continue`.
3. **P
...
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-2124671596)
Good points. I think it could work well to combine the two warnings to a single dialog. It could be placed between the initial `Create wallet` dialog, and the password input.
1. **Create wallet dialog:** The user checks `Encrypt wallet` and clicks `Continue`.
2. **Password warning dialog:** The application informs the user about the risks of using a password and offers `Back` and `Continue` options. The user considers, decides to move forward with the password, and clicks `Continue`.
3. **P
...
⚠️ epiccurious opened an issue: "Improve the bitcoin.conf instructions in init.md doc"
(https://github.com/bitcoin/bitcoin/issues/30153)
Update the `init.md` instructions about generating an example `bitcoin.conf` file.
Additionally, the `share/examples/bitcoin.conf` placeholder file says to follow the instructions in `contrib/devtools/README.md`, which was confusing to me since that README talks about many scripts. It would be helpful to point out to the user which section to refer to.
This issue will include an accompanying PR (to be assigned).
## Background
- Two years ago, `share/examples/bitcoin.conf` was replac
...
(https://github.com/bitcoin/bitcoin/issues/30153)
Update the `init.md` instructions about generating an example `bitcoin.conf` file.
Additionally, the `share/examples/bitcoin.conf` placeholder file says to follow the instructions in `contrib/devtools/README.md`, which was confusing to me since that README talks about many scripts. It would be helpful to point out to the user which section to refer to.
This issue will include an accompanying PR (to be assigned).
## Background
- Two years ago, `share/examples/bitcoin.conf` was replac
...
📝 epiccurious opened a pull request: "doc: Update mentions of generating bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30154)
Closes #30153.
This PR includes two changes as separate commits:
1. Update `doc/init.md` to mention generating an example bitcoin.conf instead of referencing the placeholder `share/examples/bitcoin.conf`. Update the code-formatted text with a markdown link.
2. Also update `share/examples/bitcoin.conf` with a mention of the appropriate section of `contrib/devtools/README.md`.
## Background
- Two years ago, `share/examples/bitcoin.conf` was replaced with [a placeholder file](https:/
...
(https://github.com/bitcoin/bitcoin/pull/30154)
Closes #30153.
This PR includes two changes as separate commits:
1. Update `doc/init.md` to mention generating an example bitcoin.conf instead of referencing the placeholder `share/examples/bitcoin.conf`. Update the code-formatted text with a markdown link.
2. Also update `share/examples/bitcoin.conf` with a mention of the appropriate section of `contrib/devtools/README.md`.
## Background
- Two years ago, `share/examples/bitcoin.conf` was replaced with [a placeholder file](https:/
...
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124675374)
If we auto-renew every 20 minutes then I guess that's fine. Maybe just add a comment somewhere that we _could_ explicitly close the mapping upon shutdown.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124675374)
If we auto-renew every 20 minutes then I guess that's fine. Maybe just add a comment somewhere that we _could_ explicitly close the mapping upon shutdown.
💬 maflcko commented on issue "Issues with new create wallet dialogue":
(https://github.com/bitcoin-core/gui/issues/151#issuecomment-2124678500)
See also https://github.com/bitcoin-core/gui/pull/722#issuecomment-2124671596
(https://github.com/bitcoin-core/gui/issues/151#issuecomment-2124678500)
See also https://github.com/bitcoin-core/gui/pull/722#issuecomment-2124671596
💬 fanquake commented on pull request "contrib: Renew Windows code signing certificate":
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2124680462)
Diff of our cert:
```diff
--- a/a.txt
+++ b/a.txt
@@ -2,12 +2,12 @@ Certificate:
Data:
Version: 3 (0x2)
Serial Number:
- 0a:65:6f:75:06:a5:ef:65:36:43:16:d4:4d:3d:d2:45
+ 07:34:78:e8:9d:b2:ab:78:3e:f8:d6:d0:4b:f0:41:54
Signature Algorithm: sha256WithRSAEncryption
Issuer: C=US, O=DigiCert, Inc., CN=DigiCert Trusted G4 Code Signing RSA4096 SHA384 2021 CA1
Validity
- Not Before: May 24 00:00:00 2022 GMT
...
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2124680462)
Diff of our cert:
```diff
--- a/a.txt
+++ b/a.txt
@@ -2,12 +2,12 @@ Certificate:
Data:
Version: 3 (0x2)
Serial Number:
- 0a:65:6f:75:06:a5:ef:65:36:43:16:d4:4d:3d:d2:45
+ 07:34:78:e8:9d:b2:ab:78:3e:f8:d6:d0:4b:f0:41:54
Signature Algorithm: sha256WithRSAEncryption
Issuer: C=US, O=DigiCert, Inc., CN=DigiCert Trusted G4 Code Signing RSA4096 SHA384 2021 CA1
Validity
- Not Before: May 24 00:00:00 2022 GMT
...
💬 fjahr commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2124683414)
Code review ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2124683414)
Code review ACK 1e54d61c4698debf3329d1960e06078ccbf8063c
✅ fanquake closed an issue: "stringop-overflow warning with GCC 14"
(https://github.com/bitcoin/bitcoin/issues/30114)
(https://github.com/bitcoin/bitcoin/issues/30114)
🚀 fanquake merged a pull request: "wallet, tests: Avoid stringop-overflow warning in PollutePubKey"
(https://github.com/bitcoin/bitcoin/pull/30131)
(https://github.com/bitcoin/bitcoin/pull/30131)
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609899492)
Getting a mismatch on the first block:
```sh
curl -s localhost:8000/tweak-index/709632?dustLimit=992 | jq
```
```json
[
"0282559668ac461407e39d78b0d604e19c88ec7d1b4eb1b07636a2fe10a00f9a74",
"0360b336c0121e9a1f64e5644a0519f2f6fd37ae28bc6558c604a63b1aac251a7c",
"03a1fbba688d7d1940910d2a40fdb08fa6c03e56ae3f1484941ae2b9451aa9672b",
"03e0b6b04bc4529c376dd8215dac77bb4d7696df34e097b28a7948686c2bc70b4c",
"02266e6da2790ffa1e47640eca18735281177b75e3435df731f41c42cc9646bdc0",
"03
...
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609899492)
Getting a mismatch on the first block:
```sh
curl -s localhost:8000/tweak-index/709632?dustLimit=992 | jq
```
```json
[
"0282559668ac461407e39d78b0d604e19c88ec7d1b4eb1b07636a2fe10a00f9a74",
"0360b336c0121e9a1f64e5644a0519f2f6fd37ae28bc6558c604a63b1aac251a7c",
"03a1fbba688d7d1940910d2a40fdb08fa6c03e56ae3f1484941ae2b9451aa9672b",
"03e0b6b04bc4529c376dd8215dac77bb4d7696df34e097b28a7948686c2bc70b4c",
"02266e6da2790ffa1e47640eca18735281177b75e3435df731f41c42cc9646bdc0",
"03
...
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609910257)
835dd4ba1ca4cc1abfd338e6bc38e25a116f5f8b: oh this is wrong, it finds the _lowest_ output amount, oops.
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609910257)
835dd4ba1ca4cc1abfd338e6bc38e25a116f5f8b: oh this is wrong, it finds the _lowest_ output amount, oops.
💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609915406)
This branch should have a sorted index. We can now find the txid a lot easier, I think.
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609915406)
This branch should have a sorted index. We can now find the txid a lot easier, I think.
💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609919080)
@TheCharlatan Thanks for playing around with it :)
I don't have the internal clang knowledge to know if that's correct or not. Looks fine to me, but I agree with @maflcko that we could just remove the empty dtor in that case.
Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp#L18
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609919080)
@TheCharlatan Thanks for playing around with it :)
I don't have the internal clang knowledge to know if that's correct or not. Looks fine to me, but I agree with @maflcko that we could just remove the empty dtor in that case.
Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp#L18
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609932428)
I added a fix for https://github.com/bitcoin/bitcoin/pull/28241/files#diff-1b4444ba5939fff2ab9f70d1d0d29fc2653b09a332e462166e01891e261c9fa9 and rebuilding the index now.
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609932428)
I added a fix for https://github.com/bitcoin/bitcoin/pull/28241/files#diff-1b4444ba5939fff2ab9f70d1d0d29fc2653b09a332e462166e01891e261c9fa9 and rebuilding the index now.
💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609936760)
Isn't there some kind of special handling needed for large values that don't fit into a byte after the shift (i.e. if txout.nValue >4080 sats)?
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609936760)
Isn't there some kind of special handling needed for large values that don't fit into a byte after the shift (i.e. if txout.nValue >4080 sats)?
💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609948787)
I'd suggest to introduce a fast `CScript::IsPayToTaproot` helper. It can be used both in `MaybeSilentPayment` and directly in the loop for finding the largest output amount (commit 337471e4acff439100488c16876d9f110ef04ef7), without the need to involve `Solver`:
<details>
<summary>patch to add `CScript::IsPayToTaproot()`</summary>
```diff
diff --git a/src/script/script.cpp b/src/script/script.cpp
index 80e8d26bcf..d0e677aaa4 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cp
...
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609948787)
I'd suggest to introduce a fast `CScript::IsPayToTaproot` helper. It can be used both in `MaybeSilentPayment` and directly in the loop for finding the largest output amount (commit 337471e4acff439100488c16876d9f110ef04ef7), without the need to involve `Solver`:
<details>
<summary>patch to add `CScript::IsPayToTaproot()`</summary>
```diff
diff --git a/src/script/script.cpp b/src/script/script.cpp
index 80e8d26bcf..d0e677aaa4 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cp
...
👍 theuni approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2071218768)
ACK fa3e1151a28345edff8f371283745bdd647f9a74
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2071218768)
ACK fa3e1151a28345edff8f371283745bdd647f9a74
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1609954791)
bebfafcf98d8ed1432407b7603991d54f7cc26c2: it would be useful to have a quick recap here of which strategy is used by `QueryDefaultGateway` for which OS.
I think it will be (slightly) more readable to have a single `QueryDefaultGateway` implementation which then calls `QueryDefaultGatewayWindows`, `QueryDefaultGatewayMac` and `QueryDefaultGatewayLinuxBSD`.
It can start with:
```cpp
Assume(network == NET_IPV4 || network == NET_IPV6)
```
and end with
```cpp
#else return std::nullo
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1609954791)
bebfafcf98d8ed1432407b7603991d54f7cc26c2: it would be useful to have a quick recap here of which strategy is used by `QueryDefaultGateway` for which OS.
I think it will be (slightly) more readable to have a single `QueryDefaultGateway` implementation which then calls `QueryDefaultGatewayWindows`, `QueryDefaultGatewayMac` and `QueryDefaultGatewayLinuxBSD`.
It can start with:
```cpp
Assume(network == NET_IPV4 || network == NET_IPV6)
```
and end with
```cpp
#else return std::nullo
...
💬 TheCharlatan commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609967684)
> I don't have freeBSD to test this, but are you sure it doesn't crash when the dtor is there but empty?
It looks like it should work to me. If a method has a body it should also contain a compound statement, or at least this [book](https://github.com/PacktPublishing/LLVM-Techniques-Tips-and-Best-Practices-Clang-and-Middle-End-Libraries) seems to suggest as much.
> Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-too
...
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609967684)
> I don't have freeBSD to test this, but are you sure it doesn't crash when the dtor is there but empty?
It looks like it should work to me. If a method has a body it should also contain a compound statement, or at least this [book](https://github.com/PacktPublishing/LLVM-Techniques-Tips-and-Best-Practices-Clang-and-Middle-End-Libraries) seems to suggest as much.
> Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-too
...
👍 AngusP approved a pull request: "contrib/signet/miner: increase miner search space"
(https://github.com/bitcoin/bitcoin/pull/30130#pullrequestreview-2071241565)
utACK 1cf174a2954b434e9623afe86f3643be8eec2d70
(https://github.com/bitcoin/bitcoin/pull/30130#pullrequestreview-2071241565)
utACK 1cf174a2954b434e9623afe86f3643be8eec2d70