💬 theStack commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145559813)
This unfortunately doesn't lead to the correct result for regtest segwit addresses, as those have the [four-character HRP 'bcrt'](https://github.com/bitcoin/bitcoin/blob/4c6b7d330a4e80460dcce19b1a0a47d77a0b99ea/src/kernel/chainparams.cpp#L510). To make it work for addresses on all networks, I'd suggest to:

1) finding the HRP by extracting the string before the bech32 separator character '1' (either the `find()` or `.split()` method on strings should be handy for this)
2) assert that the extr
...

💬 Crypto2 commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480516783)
It happened again today, sometime between 12-1pm Toronto time. If you look at the unconfirmed balance in this hourly log it just drops and it is ignoring those pending funds. I did restart the node but that didn't fix it (and no they didn't just go in to the confirmed balance.) listtransactions for that hour shows less than 1 BTC of volume in either direction.

12a BTC: wallet 0 balances -> balance: X.30494649 / unconfirmed_balance: 22.45820421
BTC: wallet 0 balances -> balance: X.01931847 /
...

💬 achow101 commented on pull request "fix: contrib: allow multi-sig binary verification":
(https://github.com/bitcoin/bitcoin/pull/23020#issuecomment-1480603300)
https://github.com/achow101/bitcoin/tree/pr23020-direct-bins-gpg-parse is a branch which implements a subcommand for verifying binaries directly and to use `--status-file` for the machine parseable output.

💬 1440000bytes commented on issue "Incorrect balance reported in getwalletinfo/getbalance":
(https://github.com/bitcoin/bitcoin/issues/21768#issuecomment-1480666670)
> I tried increasing the maxmempool to 1000 instead of the default 300 and restarted again, and now the funds are showing again

Don't use default mempool size if you have enough RAM on machine running bitcoind. Default was set years ago and doesn't make sense anymore. There are other disadvantages as well for using default or lower size mempool.

💬 S3RK commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1145824454)
q: do I understand correctly with current filters `total_discarded` is always equal to `total_unconf_long_chain`?

💬 S3RK commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#issuecomment-1480768733)
Code review ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b

✅ MarcoFalke closed an issue: "Bitcoind repeatedly crashing at `UpdateTip` with no error message"
(https://github.com/bitcoin/bitcoin/issues/24483)

💬 MarcoFalke commented on issue "contrib/install_db4.sh script fails on OpenBSD 7.2 (RPi 3) (error: Unable to find a mutex implementation)":
(https://github.com/bitcoin/bitcoin/issues/26860#issuecomment-1480787053)
Closing for now. Let us know if this is still an issue and should be reopened.

✅ MarcoFalke closed an issue: "contrib/install_db4.sh script fails on OpenBSD 7.2 (RPi 3) (error: Unable to find a mutex implementation)"
(https://github.com/bitcoin/bitcoin/issues/26860)

💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1145848255)
You could do it in a separate commit, after the move

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145852472)
Thank you for your valuable feedback.
- I will use `.split('1')` on the address to extract the hrp, and also assert to verify if the hrp is valid as you noted.
- I will have to modify `address_to_scriptpubkey ` to check for base58 address first before calling `bech32_to_bytes` or it will assert false.

I hope this addresses the issue

💬 MarcoFalke commented on pull request "refactor: Replace GetTimeMicros by SystemClock":
(https://github.com/bitcoin/bitcoin/pull/27233#discussion_r1145858622)
Yes, I am happy to review a pull doing this.

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145873825)
I'm not sure I understand your second point, @ismaelsadeeq . The order shouldn't matter in `address_to_scriptpubkey`. It should be sufficient to properly extract the HRP and check that it is a bech32.

📝 hebasto opened a pull request: "ci: Use clang-15 in "tidy" task"
(https://github.com/bitcoin/bitcoin/pull/27311)
Newer tools usually are better in terms of features and bug fixes.

Requested in https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1440230390.

Split from https://github.com/bitcoin/bitcoin/pull/26766.

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1480832579)
> I don't know much about how the CI works, maybe it's possible to install a newer clang version? I'm using 15.0.7

Done in https://github.com/bitcoin/bitcoin/pull/27311.

Making this PR a draft for now.

📝 hebasto converted_to_draft a pull request: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)
null

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145886593)
```suggestion
version, payload = bech32_to_bytes(address)
```
@josibake this will call will assert to false if the address is a base58 address.

💬 MarcoFalke commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#discussion_r1145910136)
> (also: https://github.com/llvm/llvm-project/issues/56195)

Thanks, fixed in https://github.com/llvm/llvm-project/commit/8c10256734cd47274671fcabe94f24f15ecd6209

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145918186)
`bech32_to_bytes` will be updated to handle all segwit addresses and check if hrp is valid.
```python
def bech32_to_bytes(address):
hrp = address.split('1')[0]
assert hrp in ['bc', 'tb', 'bcrt'], f"hrp should be 'bc', 'tb', or 'bcrt', not '{hrp}'"
version, payload = decode_segwit_address(hrp, address)
if version is None:
return (None, None)
return version, bytearray(payload)
```
So calling address_to_scriptpubkey with a base58 address will assert to false,
...

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145925672)
ah, i see. I think id approach this in a different way: if the HRP doesn't match a bech32 HRP, return `(None, None)`. that way, `address_to_scriptpubkey` can continue and try base58 next. if the address is indeed malformed (meaning not bech32 or base58), eventually `address_to_scriptpubkey` will return false.