✅ MarcoFalke closed an issue: "Malware reported in scan of bitcoin-qt versions 22 and 24.0.1 apple darwin"
(https://github.com/bitcoin/bitcoin/issues/28049)

💬 dsldsl commented on issue "Malware reported in scan of bitcoin-qt versions 22 and 24.0.1 apple darwin":
(https://github.com/bitcoin/bitcoin/issues/28049#issuecomment-1630168865)
that's helpful thank you

⚠️ meglio opened an issue: "Internal bug detected: "isValid == error_msg.empty()" when validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)"
(https://github.com/bitcoin/bitcoin/issues/28064)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)

when calling through rpc api, throws an error:

```
Internal bug detected: "isValid == error_msg.empty()"
rpc/output_script.cpp:64 (operator())
Bitcoin Core v25.0.0
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
```

### Expected behaviour

Address validation to return is_v
...

💬 MarcoFalke commented on issue "Internal bug detected: "isValid == error_msg.empty()" when validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)":
(https://github.com/bitcoin/bitcoin/issues/28064#issuecomment-1630193718)
This will be fixed in 25.1, see 725c3dc2dd1402d631e05ef461fe2abbac9a008f

💬 S3RK commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1259265158)
agree, you can resolve this

💬 S3RK commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#issuecomment-1630248097)
Approach ACK

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1259300260)
In theory the waste score should be the same.

Because not all of the params are available within BnB we call
`result.ComputeAndSetWaste(cost_of_change, cost_of_change, 0);`
instead of
`result.ComputeAndSetWaste(min_viable_change, cost_of_change, change_fee);`

I believe this should be fine as BnB will not produce results with more than `cost_of_change` excess and therefore `GetChange()` will always return 0 and then `change_fee` doesn't matter.

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1259306993)
Yes, the discount value is a function of the effective fee rate, therefore it has to be taken into account in waste calculation. The problem is that `GetSelectionWaste` doesn't use `SelectionResult.GetEffectiveValue()` which already accounts for bump fee discount.

💬 fanquake commented on issue "Internal bug detected: "isValid == error_msg.empty()" when validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)":
(https://github.com/bitcoin/bitcoin/issues/28064#issuecomment-1630335753)
Closing, given the fix has been backported. @meglio if you're able to compile the 25.x branch, and confirm this fixes your issue, please do.

✅ fanquake closed an issue: "Internal bug detected: "isValid == error_msg.empty()" when validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)"
(https://github.com/bitcoin/bitcoin/issues/28064)

💬 russeree commented on issue "Internal bug detected: "isValid == error_msg.empty()" when validateaddress(bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf)":
(https://github.com/bitcoin/bitcoin/issues/28064#issuecomment-1630337550)
This is currently resolved in master via the addition of an else case to ```if (ConvertBits<5, 8, false>([&](unsigned char c) { data.push_back(c); }, dec.data.begin() + 1, dec.data.end()))```

here

https://github.com/bitcoin/bitcoin/blob/ef29d5d7e239b42269dd22ea94a709b5e4ceb5e5/src/key_io.cpp#L194-L197

💬 russeree commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1259398907)
1. This should not have a period '.' at the end. Since it is inconsistent with the rest of the documentation.
2. Should start with a lowercase letter since only grammatical articles have capitalized first letters for this documentation. (A, The, This)
3. The language used in BIP22 is a more concise definition ```"longpollid" of job to monitor for expiration; required and valid only for long poll requests```

🤔 russeree requested changes to a pull request: "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998"
(https://github.com/bitcoin/bitcoin/pull/28056#pullrequestreview-1523750256)
Some of my thoughts about this PR. Mostly regarding the grammatical consistency of these messages.

💬 russeree commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1259399690)
Should start with a lowercase letter since only grammatical articles have capitalized first letters for this documentation. (A, The, This). Similar to review item number one.

💬 russeree commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1259401850)
1. This should not have a period '.' at the end. Since it is inconsistent with the rest of the documentation.
2. Should start with a lowercase letter since only grammatical articles have capitalized first letters for this documentation. (A, The, This)
3. IMO The language used in BIP22 is a more concise definition "longpollid" of job to monitor for expiration; required and valid only for long poll requests. Others please chime in.

💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1259409790)
Oh yes, I agree the descriptions should have the same grammatical style as the rest. I can make these changes for sure. Thanks @russeree for the help :D

💬 russeree commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#discussion_r1259413067)
No problem, this is what I got running your code.


🚀 fanquake merged a pull request: "test: Check expected_stderr after stop"
(https://github.com/bitcoin/bitcoin/pull/28028)

💬 TheCharlatan commented on pull request "refactor: Move stopafterblockimport option out of blockstorage":
(https://github.com/bitcoin/bitcoin/pull/28053#issuecomment-1630478608)
Rebased 5867bb1ae7d1d345a5fd3b341d88ff777b6ef404 -> f87c21c1827fa532749ec17a2f44322378046b93 ([blockImportReturn_0](https://github.com/TheCharlatan/bitcoin/tree/blockImportReturn_0) -> [blockImportReturn_1](https://github.com/TheCharlatan/bitcoin/tree/blockImportReturn_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockImportReturn_0..blockImportReturn_1))

* Fixed conflict with #27607.
* Dropped boolean argument for making the function return early.

💬 MarcoFalke commented on pull request "ci: build Valgrind (3.21) from source":
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1630513681)
Well, it would be be good to exactly explain what this change is trying to solve and then explain why it solves the issue.

If you are trying to fix the slowness, a better fix may be to just apply `-O3` instead of `-O0` to *Bitcoin Core* and leave valgrind alone, as failing to apply optimizations is not a valgrind bug.

If you are trying to fix something else, it may be good to explain as well.