💬 maflcko commented on pull request "doc: fix broken relative md links":
(https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2097553185)
ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3

💬 maflcko commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2097557892)
Did anyone that could reproduce, try to reproduce in gdb? https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2089900690

💬 maflcko commented on issue "test: Intermittent issue in feature_init.py", line 88, in run_test with node.wait_for_debug_log([terminate_line]): AssertionError: [node 0] Expected messages "[b'scheduler thread start']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30011#issuecomment-2097572483)
Possibly it started to happen after https://github.com/bitcoin/bitcoin/pull/29848 ?

💬 maflcko commented on issue "test: Intermittent issue in feature_init.py", line 88, in run_test with node.wait_for_debug_log([terminate_line]): AssertionError: [node 0] Expected messages "[b'scheduler thread start']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30011#issuecomment-2097573565)
ugh https://github.com/bitcoin/bitcoin/pull/29848#discussion_r1563849606

📝 maflcko opened a pull request: "ci: Exclude feature_init for now in valgrind task"
(https://github.com/bitcoin/bitcoin/pull/30054)
Fixes https://github.com/bitcoin/bitcoin/issues/30011

💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1591963853)
Added a comment for where the point comes from. Tried to write a sentence for the "why" but don't really understand it well enough to articulate succinctly :sweat_smile:.

From what I understand, because `H` is derived from a known public value `G` via a hash, we learn nothing about the discrete log of `H` due to the hash function providing us a uniformly random value. But not sure if that's entirely correct; if you have a suggestion on how to better (or more correctly!) phrase this, happy to
...

💬 maflcko commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1591973597)
Any reason to drop the ParseHex, which allows to write this as a string literal?

🤔 ismaelsadeeq reviewed a pull request: "doc: fix broken relative md links"
(https://github.com/bitcoin/bitcoin/pull/30025#pullrequestreview-2042354354)
Re ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3

💬 ajtowns commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1592014682)
Oh, secp256k1-zkp has a straight forward description: https://github.com/BlockstreamResearch/secp256k1-zkp/blob/11af7015de624b010424273be3d91f117f172c82/src/modules/rangeproof/main_impl.h#L16

💬 vasild commented on issue "Could the wallet count unconfirmed non-mempool change?":
(https://github.com/bitcoin/bitcoin/issues/11887#issuecomment-2097746943)
> Related to private broadcast @vasild ?

Right. Mentioned at the bottom of OP in https://github.com/bitcoin/bitcoin/pull/29415:

> Next step after this PR is done will be to have the wallet do the private broadcast as well, https://github.com/bitcoin/bitcoin/issues/11887 would have to be resolved.

💬 maflcko commented on pull request "build: Require libc++-16 or later":
(https://github.com/bitcoin/bitcoin/pull/29077#issuecomment-2097780769)
Not sure if there is much value in being overly accurate here (`clang-15 and libc++-16` or `clang-16 and libstdc++`). If someone can install clang-16 or libc++-16, they can probably also install the other as well?

So I'd say to close this and bump to 16 wholesale?

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1592074394)
:doh: yes! fixed and added unit tests for the `NO_LIMIT` case.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2097797401)
Rebased for #29984

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1592075348)
Yes, good point.

✅ vasild closed a pull request: "addrman: treat Tor/I2P/CJDNS as a single group"
(https://github.com/bitcoin/bitcoin/pull/22563)

💬 vasild commented on pull request "addrman: treat Tor/I2P/CJDNS as a single group":
(https://github.com/bitcoin/bitcoin/pull/22563#issuecomment-2097799927)
Closing this because I think the proper solution is a bigger change as mentioned above in https://github.com/bitcoin/bitcoin/pull/22563#issuecomment-1717157204:

_If the address is IPv4 or IPv6 then use `GetGroup()`
Else do something else (for Tor, I2P and CJDNS addresses)._

Currently `GetGroup()` is called for all types of addresses from:
`AddrInfo::GetTriedBucket()`
`AddrInfo::GetNewBucket()`
`CConnman::CalculateKeyedNetGroup() // saved in CNode::nKeyedNetGroup and later used during e
...

💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1592086885)
Perfect, will use this!

💬 josibake commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1592095735)
Seemed unnecessary? Representing this as a byte vector allows us to create an `XOnlyPublicKey` directly without an extra function call. Is there a reason to prefer having this as a string literal?

💬 hebasto commented on pull request "build, test, doc: Temporarily remove Android-related stuff":
(https://github.com/bitcoin/bitcoin/pull/30049#issuecomment-2097841736)
> I guess the idea is that there's basically nothing worth salvaging from our current Android build procedure?

The commit history keeps everything we might need when restoring Android builds.

💬 maflcko commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#discussion_r1592108480)
Just a style-nit, as it makes it easier to grep and compare the string literal. The function call could be moved to compile time, but I haven't implemented it yet. Calling it once at runtime should be fine also, for now?

Just a nit in any case.