💬 Sjors commented on pull request "wallet: Always set descriptor cache upgraded flag for new wallets":
(https://github.com/bitcoin/bitcoin/pull/32597#issuecomment-2944074815)
ACK 47237cd1938058b29fdec242c3a37611e255fda0
(https://github.com/bitcoin/bitcoin/pull/32597#issuecomment-2944074815)
ACK 47237cd1938058b29fdec242c3a37611e255fda0
👍 theStack approved a pull request: "test: wallet: cover wallet passphrase with a null char"
(https://github.com/bitcoin/bitcoin/pull/32675#pullrequestreview-2900231176)
ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064
(https://github.com/bitcoin/bitcoin/pull/32675#pullrequestreview-2900231176)
ACK 7cfbb8575e1ffbad5c48e2c461b45dd6ac63d064
💬 theStack commented on pull request "test: wallet: cover wallet passphrase with a null char":
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2128796675)
nit: as there is already a null character inside the string, so could just append a regular character at the end to cause a mismatch with this error message
```suggestion
assert_raises_rpc_error(-14, "wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrase, passphrase_with_nulls + "x", 10)
brunoerg marked this conversation as resolved.
assert_raises_rpc_error(-14, "The old wallet passphrase entered is incorrect. It cont
...
(https://github.com/bitcoin/bitcoin/pull/32675#discussion_r2128796675)
nit: as there is already a null character inside the string, so could just append a regular character at the end to cause a mismatch with this error message
```suggestion
assert_raises_rpc_error(-14, "wallet passphrase entered is incorrect. It contains a null character (ie - a zero byte)", self.nodes[0].walletpassphrase, passphrase_with_nulls + "x", 10)
brunoerg marked this conversation as resolved.
assert_raises_rpc_error(-14, "The old wallet passphrase entered is incorrect. It cont
...
⚠️ Fuzion24 opened an issue: "Internal bug detected: Fee needed > fee paid"
(https://github.com/bitcoin/bitcoin/issues/32683)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
walletcreatefundedpsbt errors out
```
walletcreatefundedpsbt "[]" '{"<readacted_addr>":0.01}'
error code: -4
error message:
Internal bug detected: Fee needed > fee paid
./wallet/spend.cpp:1320 (CreateTransactionInternal)
Bitcoin Core v29.0.0
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
```
### Expected behaviour
walletcreatefundedpsbt should not error out.
...
(https://github.com/bitcoin/bitcoin/issues/32683)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
walletcreatefundedpsbt errors out
```
walletcreatefundedpsbt "[]" '{"<readacted_addr>":0.01}'
error code: -4
error message:
Internal bug detected: Fee needed > fee paid
./wallet/spend.cpp:1320 (CreateTransactionInternal)
Bitcoin Core v29.0.0
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
```
### Expected behaviour
walletcreatefundedpsbt should not error out.
...
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2128825473)
I raised a PR for it here: https://github.com/bitcoin/bips/pull/1866
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2128825473)
I raised a PR for it here: https://github.com/bitcoin/bips/pull/1866
🤔 theStack reviewed a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-2900301627)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-2900301627)
Concept ACK
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2128836472)
in commit 5e90a3c747ac2d44f9f52a3eeffbbe391f837bd8: seems like this change should be part of the earlier commit 01f639835783aa10d4c4b63da477c0300bc8495f, which currently doesn't compile
```
...
In file included from /home/thestack/bitcoin_prrev/pr29675/src/musig.cpp:5:
/home/thestack/bitcoin_prrev/pr29675/src/musig.h:44:5: error: no template named 'secure_unique_ptr'
secure_unique_ptr<secp256k1_musig_secnonce> m_nonce;
/home/thestack/bitcoin_prrev/pr29675/src/musig.cpp:74:36: error:
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2128836472)
in commit 5e90a3c747ac2d44f9f52a3eeffbbe391f837bd8: seems like this change should be part of the earlier commit 01f639835783aa10d4c4b63da477c0300bc8495f, which currently doesn't compile
```
...
In file included from /home/thestack/bitcoin_prrev/pr29675/src/musig.cpp:5:
/home/thestack/bitcoin_prrev/pr29675/src/musig.h:44:5: error: no template named 'secure_unique_ptr'
secure_unique_ptr<secp256k1_musig_secnonce> m_nonce;
/home/thestack/bitcoin_prrev/pr29675/src/musig.cpp:74:36: error:
...
💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2944333990)
@Zeegaths I think you can wait for 24hrs if still no response from @fanquake you can go ahead and start a draft PR. I can help you review it when you finalize it.
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2944333990)
@Zeegaths I think you can wait for 24hrs if still no response from @fanquake you can go ahead and start a draft PR. I can help you review it when you finalize it.
💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2944359326)
I wanted to do it but I am currently trying to review a very interesting PR 😊.
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2944359326)
I wanted to do it but I am currently trying to review a very interesting PR 😊.
💬 liamperfil commented on pull request "gui: Add a menu action to restore then migrate a legacy wallet":
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2944369692)
I support the proposal, but I also consider that this extra click is a bad experience for the user. The ideal would be to directly integrate this functionality to automatically detect if it is a legacy wallet and offer the migration option in the same flow. If you need someone on your team I am available.
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2944369692)
I support the proposal, but I also consider that this extra click is a bad experience for the user. The ideal would be to directly integrate this functionality to automatically detect if it is a legacy wallet and offer the migration option in the same flow. If you need someone on your team I am available.
🚀 fanquake merged a pull request: "[28.x] Backport #31407"
(https://github.com/bitcoin/bitcoin/pull/32563)
(https://github.com/bitcoin/bitcoin/pull/32563)
💬 Sjors commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2128878830)
@theStack it only runs the most recent N (6?) commits.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2128878830)
@theStack it only runs the most recent N (6?) commits.
📝 fanquake opened a pull request: "[28.x] 28.2rc2"
(https://github.com/bitcoin/bitcoin/pull/32684)
Backports #32568.
Bumps to `rc2`.
Also includes #32563 & #32639 since `rc1`.
(https://github.com/bitcoin/bitcoin/pull/32684)
Backports #32568.
Bumps to `rc2`.
Also includes #32563 & #32639 since `rc1`.
💬 fanquake commented on pull request "depends: use "mkdir -p" when installing xproto":
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2944484421)
Backported to `28.x` in #32684.
(https://github.com/bitcoin/bitcoin/pull/32568#issuecomment-2944484421)
Backported to `28.x` in #32684.
💬 John-zhan commented on issue "windows: depends config fails":
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2944494245)
I have do as https://github.com/microsoft/vcpkg/issues/45814 and now cmake successfully config, and build bitcoin success.
(https://github.com/bitcoin/bitcoin/issues/32578#issuecomment-2944494245)
I have do as https://github.com/microsoft/vcpkg/issues/45814 and now cmake successfully config, and build bitcoin success.
✅ John-zhan closed an issue: "windows: depends config fails"
(https://github.com/bitcoin/bitcoin/issues/32578)
(https://github.com/bitcoin/bitcoin/issues/32578)
💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2128930669)
In [BIP 327](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki), the paragraph starting from [here](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki#identifying-disruptive-signers:~:text=The%20same%20individual%20public%20key%20is%20allowed%20to%20occur%20more%20than%20once%20in%20the%20input%20of%20KeyAgg%20and%20KeySort.) makes the case for allowing duplicate keys in the protocol. It's also recommended to omit such checks.
> In fact, applications are recommended
...
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2128930669)
In [BIP 327](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki), the paragraph starting from [here](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki#identifying-disruptive-signers:~:text=The%20same%20individual%20public%20key%20is%20allowed%20to%20occur%20more%20than%20once%20in%20the%20input%20of%20KeyAgg%20and%20KeySort.) makes the case for allowing duplicate keys in the protocol. It's also recommended to omit such checks.
> In fact, applications are recommended
...
🤔 vasild reviewed a pull request: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2899730748)
Posting review midway. Reviewed up to and including b90f808e30 `http: disconnect after idle timeout (-rpcservertimeout)`
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2899730748)
Posting review midway. Reviewed up to and including b90f808e30 `http: disconnect after idle timeout (-rpcservertimeout)`
💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2128494930)
Would be good to have tests to exercise the chunked transfer encoding from d7778f426c `http: support "chunked" Transfer-Encoding`.
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2128494930)
Would be good to have tests to exercise the chunked transfer encoding from d7778f426c `http: support "chunked" Transfer-Encoding`.
💬 vasild commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2128594514)
The `CloseConnectionInternal()` method can and should take just a reference, no need for shared pointer:
```diff
- void HTTPServer::CloseConnectionInternal(std::shared_ptr<HTTPClient>& client)
+ void HTTPServer::CloseConnectionInternal(const HTTPClient& client)
```
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2128594514)
The `CloseConnectionInternal()` method can and should take just a reference, no need for shared pointer:
```diff
- void HTTPServer::CloseConnectionInternal(std::shared_ptr<HTTPClient>& client)
+ void HTTPServer::CloseConnectionInternal(const HTTPClient& client)
```