🤔 ismaelsadeeq reviewed a pull request: "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/28027#pullrequestreview-1523177321)
Running this test failed on my device not sure if it's from this PR though.

```
2023-07-11T00:04:00.468000Z TestFramework (INFO): PRNG seed is: 1124432296273551722
2023-07-11T00:04:00.468000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_banbrcm1
2023-07-11T00:04:00.472000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/abubakarsadiq/Desktop/bitcoin/test/functional/test_framework/test_framework.py", line 550, in start_n
...

💬 ismaelsadeeq 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_r1259010439)
The v19 test pubkeys `0296b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52` and `037211a824f55b505228e4c3d5194c1fcfaa15a456abdf37f9b9d97a4040afc073` are they relevant to the test? I see they are the same as the one reported in #18075

💬 ismaelsadeeq 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_r1259015544)
Why are we no longer using the wallet?

💬 ismaelsadeeq 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_r1259013655)
`180000`, `210000`, e.t.c
To reduce the magic numbers we can create a variable for the versions, e.g `v18 = 180000`and reuse it throughout the tests?

💬 ariard commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1259064139)
So `m_recent_rejects_reconsiderable` logs transaction on its wtxid (which is good to avoid witness inflation changing the result) however isn’t that two restrictive if you have PKG1 submitted with A+B, A is too low with B and then you have A+B’ where A is good enough ? Or is that `TX_LOW_FEE` when the transaction isn’t good enough to meet “mempool min fee not meet” / “min relay fee not met” ?

If it’s correct, I would still favor to exempt package transaction from min relay fee checks as it’s
...

✅ fanquake closed a pull request: "Create generator-generic-ossf-slsa3-publish.yml"
(https://github.com/bitcoin-core/gui/pull/746)

📝 fanquake locked a pull request: "Create generator-generic-ossf-slsa3-publish.yml"
(https://github.com/bitcoin-core/gui/pull/746)

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that impr
...

💬 MarcoFalke commented on pull request "RPC: Add universal options argument to listtransactions":
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1630123482)
I think you'll still need to rebase or fix the compile error

💬 MarcoFalke 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-1630141468)
See https://github.com/bitcoin/bitcoin/issues/17779 for further steps

✅ 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