💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202900053)
Yes, that follows from my previous comment. I note that this works however the distribution will not be truly random since the chance of selecting a large UTXO is greatest the first time and so on.
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202900053)
Yes, that follows from my previous comment. I note that this works however the distribution will not be truly random since the chance of selecting a large UTXO is greatest the first time and so on.
💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202900649)
Maybe randomize the pool once it's created so there is equal likely-hood of having a large UTXO at the back of the list.
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202900649)
Maybe randomize the pool once it's created so there is equal likely-hood of having a large UTXO at the back of the list.
💬 sipa commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202902938)
scriptPubKeys of size above 10000 are consensus-unspendable.
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2202902938)
scriptPubKeys of size above 10000 are consensus-unspendable.
💬 tnndbtc commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3066264494)
@maflcko For "stdin is missing", I was referring to a suspicion that bitcoind failed to write an empty string to the pipe and caused signer.py waiting on `buffer = sys.stdin.read()`. However, I was wrong about that.
After I checked the call stack posted in https://github.com/bitcoin/bitcoin/issues/32524 , it indicated that bitcoind got stuck on a `read()` call from err_rd_pipe in `util::read_atmost_n`, which is to read from the child process, i.e., signer.py.
Also, the bitcoind process indeed
...
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3066264494)
@maflcko For "stdin is missing", I was referring to a suspicion that bitcoind failed to write an empty string to the pipe and caused signer.py waiting on `buffer = sys.stdin.read()`. However, I was wrong about that.
After I checked the call stack posted in https://github.com/bitcoin/bitcoin/issues/32524 , it indicated that bitcoind got stuck on a `read()` call from err_rd_pipe in `util::read_atmost_n`, which is to read from the child process, i.e., signer.py.
Also, the bitcoind process indeed
...
💬 Sammie05 commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3066331603)
I checked this branch locally and saw that it removes -Werror=dev from the CMake generation step.
Just to understand better: is this to prevent CI from failing on dev warnings? Looks reasonable to me, but curious if it’s meant to be temporary or permanent.
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3066331603)
I checked this branch locally and saw that it removes -Werror=dev from the CMake generation step.
Just to understand better: is this to prevent CI from failing on dev warnings? Looks reasonable to me, but curious if it’s meant to be temporary or permanent.
💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2203049928)
> In contrast to the existing fuzz test, this fuzz target generates inputs that have solutions and don’t have solution. It then asserts that when the brute force search indicates that a solution exists
On second thought, I don't see the value in a test that tests for results that "don't have solutions", The valuable test is testing the quality of any produced solution. Therefore, I'm not sure what the value is to using a brute force method to show that no solution is produced.
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2203049928)
> In contrast to the existing fuzz test, this fuzz target generates inputs that have solutions and don’t have solution. It then asserts that when the brute force search indicates that a solution exists
On second thought, I don't see the value in a test that tests for results that "don't have solutions", The valuable test is testing the quality of any produced solution. Therefore, I'm not sure what the value is to using a brute force method to show that no solution is produced.
💬 Sammie05 commented on pull request "doc: add note for watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32866#issuecomment-3066363528)
Checked this branch locally. the added note clarifies what happens if the legacy wallet only contains watch-only scripts.
It helps make migration behavior clearer for users. it actually Looks good to me!
Also noticed the LLM linter suggested rewording ‘which the wallet knows but is not watching…’ maybe worth considering for clarity.
Thanks for adding this
(https://github.com/bitcoin/bitcoin/pull/32866#issuecomment-3066363528)
Checked this branch locally. the added note clarifies what happens if the legacy wallet only contains watch-only scripts.
It helps make migration behavior clearer for users. it actually Looks good to me!
Also noticed the LLM linter suggested rewording ‘which the wallet knows but is not watching…’ maybe worth considering for clarity.
Thanks for adding this
💬 Sammie05 commented on pull request "test: Enhance GetTxSigOpCost tests for coinbase transactions":
(https://github.com/bitcoin/bitcoin/pull/32840#issuecomment-3066420791)
Thanks for adding these assertions!
I like how you explicitly test that coinbase witnesses don’t contribute to SigOp cost.
Makes it clearer for future maintainers.
LGTM 👍
(https://github.com/bitcoin/bitcoin/pull/32840#issuecomment-3066420791)
Thanks for adding these assertions!
I like how you explicitly test that coinbase witnesses don’t contribute to SigOp cost.
Makes it clearer for future maintainers.
LGTM 👍
💬 Sammie05 commented on pull request "test: add logging to mock external signers":
(https://github.com/bitcoin/bitcoin/pull/32928#issuecomment-3066460410)
Nice refactor! Moving the mock helpers and log formatter to utils makes things clearer and easier to reuse.
And also logging directly into test_framework.log feels like a practical way to debug signer issues that stdout/stderr can’t catch.
Thanks for making this more maintainable
(https://github.com/bitcoin/bitcoin/pull/32928#issuecomment-3066460410)
Nice refactor! Moving the mock helpers and log formatter to utils makes things clearer and easier to reuse.
And also logging directly into test_framework.log feels like a practical way to debug signer issues that stdout/stderr can’t catch.
Thanks for making this more maintainable
💬 Sammie05 commented on pull request "test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded)":
(https://github.com/bitcoin/bitcoin/pull/32924#issuecomment-3066510113)
Thanks for adding coverage for this very specific edge case.
Using a real testnet transaction is a solid way to ensure the test reflects actual chain data.
Also appreciate the clear comments explaining why this minimal 8-byte DER signature can appear and why it matters.
(https://github.com/bitcoin/bitcoin/pull/32924#issuecomment-3066510113)
Thanks for adding coverage for this very specific edge case.
Using a real testnet transaction is a solid way to ensure the test reflects actual chain data.
Also appreciate the clear comments explaining why this minimal 8-byte DER signature can appear and why it matters.
📝 Sammie05 opened a pull request: "doc: clarify note about backup after migratewallet"
(https://github.com/bitcoin/bitcoin/pull/32956)
Added a clarifying note that after migration, the original legacy wallet file remains unchanged on disk as a backup. This helps users understand that migration is non-destructive.
(https://github.com/bitcoin/bitcoin/pull/32956)
Added a clarifying note that after migration, the original legacy wallet file remains unchanged on disk as a backup. This helps users understand that migration is non-destructive.
💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2203147913)
Thank you for the detailed review!
I think you're right, my current approach regarding named parameter handling is too verbose/complex, and I looked at your implementation, and I really liked the way you're handling named parameters in the RPCConvertNamedValues function. I will implement that approach in my next update commit. Besides that, I also looked at the json parsing issue you're mentioning, and it's easily reproducible. I experimented with both the approaches, the one you provided and
...
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2203147913)
Thank you for the detailed review!
I think you're right, my current approach regarding named parameter handling is too verbose/complex, and I looked at your implementation, and I really liked the way you're handling named parameters in the RPCConvertNamedValues function. I will implement that approach in my next update commit. Besides that, I also looked at the json parsing issue you're mentioning, and it's easily reproducible. I experimented with both the approaches, the one you provided and
...
💬 Sjors commented on issue "Guix build fails on M4 macOS host with Ubuntu in UTM":
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3066701981)
So far I found:
- `python-scikit-build-core` builds fine
- `python-pydantic-2` fails
- `python-pydantic-core` fails
My current suspicion is that `Python-2.7.18` / `python2-minimal` is the culprit, based on what was being built around the time it fails.
However I'm unable to produce the failure if I just insert `python-2.7` somewhere in the manifest. Part of the problem is that I don't really understand how these manifest files work.
But if my suspicion is current, then it might be a matter o
...
(https://github.com/bitcoin/bitcoin/issues/32759#issuecomment-3066701981)
So far I found:
- `python-scikit-build-core` builds fine
- `python-pydantic-2` fails
- `python-pydantic-core` fails
My current suspicion is that `Python-2.7.18` / `python2-minimal` is the culprit, based on what was being built around the time it fails.
However I'm unable to produce the failure if I just insert `python-2.7` somewhere in the manifest. Part of the problem is that I don't really understand how these manifest files work.
But if my suspicion is current, then it might be a matter o
...
💬 soosho commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3066998502)
uo
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3066998502)
uo
💬 pinheadmz commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3066999923)
What's the issue here? What do you consider "IBD" and "normal sync"?
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3066999923)
What's the issue here? What do you consider "IBD" and "normal sync"?
💬 kiss1111kiss commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067049412)
你好你得问题解决了吗 我也目前也是这样得情况
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067049412)
你好你得问题解决了吗 我也目前也是这样得情况
💬 kiss1111kiss commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067049539)
Hey, my litecoin wallet is saying its corrupted when I’m trying to send funds. I do have passphrase on it and I get this error while trying to send out litecoin. Also, when trying to change my passphrase litecoin core will close when i try and change my passphrase. Any help with this would be great and I appreciate your time. thanks Reward 10ltc
<img width="705" height="248" alt="Image" src="https://github.com/user-attachments/assets/0ab36f99-e6f3-4ee5-ba66-002a6ff4001f" />
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067049539)
Hey, my litecoin wallet is saying its corrupted when I’m trying to send funds. I do have passphrase on it and I get this error while trying to send out litecoin. Also, when trying to change my passphrase litecoin core will close when i try and change my passphrase. Any help with this would be great and I appreciate your time. thanks Reward 10ltc
<img width="705" height="248" alt="Image" src="https://github.com/user-attachments/assets/0ab36f99-e6f3-4ee5-ba66-002a6ff4001f" />
💬 kiss1111kiss commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067050259)
Hey, my litecoin wallet is saying its corrupted when I’m trying to send funds. I do have passphrase on it and I get this error while trying to send out litecoin. Also, when trying to change my passphrase litecoin core will close when i try and change my passphrase. Any help with this would be great and I appreciate your time. thanks Reward 10ltc
<img width="705" height="248" alt="Image" src="https://github.com/user-attachments/assets/d9a1ee7f-206b-45ea-8afa-a06160409637" />
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-3067050259)
Hey, my litecoin wallet is saying its corrupted when I’m trying to send funds. I do have passphrase on it and I get this error while trying to send out litecoin. Also, when trying to change my passphrase litecoin core will close when i try and change my passphrase. Any help with this would be great and I appreciate your time. thanks Reward 10ltc
<img width="705" height="248" alt="Image" src="https://github.com/user-attachments/assets/d9a1ee7f-206b-45ea-8afa-a06160409637" />
⚠️ dooglus opened an issue: "SegFault in QSortFilterProxyModelPrivate::build_source_to_proxy_mapping"
(https://github.com/bitcoin/bitcoin/issues/32957)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I was loading a bunch of old wallets sequentially using "bitcoin-cli loadwallet" in a loop. bitcoin-qt crashed shortly after finishing loading one wallet and starting to load the next.
The debug log shows this at the end:
2025-07-13T13:06:26Z init message: Rescanning…
2025-07-13T13:06:26Z [wallet23] Rescanning last 3130 blocks (from block 902235)...
2025-07-13T13:06:26Z [wall
...
(https://github.com/bitcoin/bitcoin/issues/32957)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I was loading a bunch of old wallets sequentially using "bitcoin-cli loadwallet" in a loop. bitcoin-qt crashed shortly after finishing loading one wallet and starting to load the next.
The debug log shows this at the end:
2025-07-13T13:06:26Z init message: Rescanning…
2025-07-13T13:06:26Z [wallet23] Rescanning last 3130 blocks (from block 902235)...
2025-07-13T13:06:26Z [wall
...
📝 kevkevinpal opened a pull request: "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult"
(https://github.com/bitcoin/bitcoin/pull/32958)
### Description
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092030045 and https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092035407
### What this changes
- Updates `PSBTError` to `PSBTResult`
because we now have `PSBTResult::OK`
- Updates `PSBTErrorString` to `PSBTResultString`
- Updates `RPCErrorFromPSBTError` to `RPCErrorFromPSBTResult`
- Removes `std::optional<common::PSBTResult>` and instead returns just `common::PSBTResult`
re
...
(https://github.com/bitcoin/bitcoin/pull/32958)
### Description
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092030045 and https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092035407
### What this changes
- Updates `PSBTError` to `PSBTResult`
because we now have `PSBTResult::OK`
- Updates `PSBTErrorString` to `PSBTResultString`
- Updates `RPCErrorFromPSBTError` to `RPCErrorFromPSBTResult`
- Removes `std::optional<common::PSBTResult>` and instead returns just `common::PSBTResult`
re
...