💬 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.

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1480921094)
Concept ACK

The code as of 8bdea6316c looks ok, but it seems to be mixing the fix with an optimization to only parse once (instead of every time a parameter is retrieved). It would be better to have two separate commits - one with the non-functional optimization and one with the fix.

Idea: I think it would be better to disallow the creation of `HTTPRequest` if the request is invalid/unparsable. This would avoid having a `nullptr` member `m_uri_parsed` and having to handle the nullness. Wou
...

🚀 fanquake merged a pull request: "refactor: Replace GetTimeMicros by SystemClock"
(https://github.com/bitcoin/bitcoin/pull/27233)

💬 fanquake commented on pull request "refactor: Replace GetTimeMicros by SystemClock":
(https://github.com/bitcoin/bitcoin/pull/27233#discussion_r1145966325)
@john-moffett want to open a follow up?

📝 fanquake opened a pull request: "depends: qrencode 4.1.1"
(https://github.com/bitcoin/bitcoin/pull/27312)
Upgrade to the latest qrencode, and disable some warnings that cause compile failures with newer compilers (clang-15+).

I haven't tested this (from a GUI perspective) at all. This is just "good enough" to keep things compiling, and uses some similar work-arounds as we have with other older packages, i.e bdb.

Note that upstream, libqrencode is effectively unmaintained. No code changes for > 2 years. No responses to issues/PRs. Seems like the author has mostly dropped off of GitHub as well.
...

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1145996614)
That will work also, thanks @josibake

💬 MarcoFalke commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1480974024)
unrelated: on master it looks like @DrahtBot can't build for Windows:


```
INFO: Building 6e69fead2baf for platform triple x86_64-w64-mingw32:
...using reference timestamp: 1679478521
...running at most 4 jobs
...from worktree directory: '/root/temp/scratch/guix/bitcoin/bitcoin'
...bind-mounted in container to: '/bitcoin'
...in build directory: '/root/temp/scratch/guix/bitcoin/bitcoin/guix-build-6e69fead2baf/distsrc-6e69fead2baf-x86_64-w64-mingw32'

...

💬 MarcoFalke commented on pull request "refactor: Replace GetTimeMicros by SystemClock":
(https://github.com/bitcoin/bitcoin/pull/27233#discussion_r1146019853)
gmtime can also be removed once we have C++20, but not sure if and when that happens

💬 willcl-ark commented on issue "tracepoints: gnu-zero-variadic-macro-arguments warnings":
(https://github.com/bitcoin/bitcoin/issues/26916#issuecomment-1480997053)
This is still present with https://github.com/bitcoin/bitcoin/pull/26593 as it also uses an underlying variadic macro with 0 arguments via `_SDT_ASM_BODY` -> `_SDT_PROBE` -> `_SDT_PROBE_N` -> `STAP_PROBEV` -> `TRACEPOINT`.

I tried to directly call the `STAP_PROBE1` macro for the single argument case (on top of #26593), which would mean that all our remaining `TRACEPOINT` macros were invoked with >= 1 argument:

```cpp
// A USDT tracepoint with one argument.
#define TRACEPOINT1(context, ev
...

💬 dergoegge commented on pull request "ci: Use clang-15 in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/27311#issuecomment-1480997560)
clang-16 was released recently, any reason not to switch to that directly while we're at it?

💬 fanquake commented on pull request "depends: fontconfig 2.14.2":
(https://github.com/bitcoin/bitcoin/pull/27301#issuecomment-1481002005)
> unrelated: on master it looks like @DrahtBot can't build for Windows:

Can @DrahtBot do a `guix pull` and try buildign again? iirc this is a bug in Guix that I've seen previously