💬 baycclark commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#discussion_r1806050734)
I don’t know where else to go for help, but this sounds like exactly what I’ve been looking for (hopefully). About a year ago I found my old pre-2017 BTC info & when viewing it I accidentally imported it as a WatchOnly & now I can’t do anything with it. I’m a Ethereum Validator & never really understood the BTC Ecosystem, so any help in the right direction would be appreciated. I’ll donate 0.02 BTC to whoever can help.

💬 ariard commented on issue "Follow-up from Moderation Rules":
(https://github.com/bitcoin/bitcoin/issues/31110#issuecomment-2421747490)
Been a while I’ve not been on IRC to attend one of the regular meeting, so I’ll find time to do so during the coming weeks to clarify. There is no rush to clarify that.

After all, it might be just a contributor or maintainer with perms access to the bitcoin Github organization. She or he might have got drunk or be under drugs at some party, and slips the wrong button….

Still hope it’s not a real infrastructure issue, where bitcoin Github repository has been effectively compromised :/

💬 virtu commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2421781383)
Concept ACK

Using a default port defeats some parts of v2transport, I guess.

Concerning DNS seeds, two ideas come to mind:
1. In theory, the issue could be solved by increasing the scope of #31062 by adding port numbers to the encoding and encode also IP addresses with it. Whether the proposal is practical remains to be seen.
2. A more radical solution would be to dispense with DNS seeds and instead rely entirely only on hardcoded seeds for all network types. [Historical data](https://de
...

💬 junghyun0783 commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2421830756)
I think I'm currently using testnet3.
How do I patch to testnet4?
When I type bitcoind -version,
"Bitcoin Core version v28.0.0"
This is what I get.
I want to apply it to testnet4 and send it.
How can I change it to testnet4?
plz help me!!!

✅ willcl-ark closed an issue: "Follow-up from Moderation Rules"
(https://github.com/bitcoin/bitcoin/issues/31110)

💬 willcl-ark commented on issue "Follow-up from Moderation Rules":
(https://github.com/bitcoin/bitcoin/issues/31110#issuecomment-2421871938)
> Just to be sure this is not a compromised of the github platform itself:

I am not myself aware of any compromise of this platform, but we can wait for others to chime in on your issue at https://github.com/bitcoin-core/meta/issues/11

I have hidden your second comment as "off-topic", as it does not align with our moderation guidelines. Specifically:
- It's off-topic and doesn't contribute to technical discussion (that applies to this entire issue frankly).
- It speculates about individu
...

⚠️ sdaftuar opened an issue: "Trouble fuzzing on macos"
(https://github.com/bitcoin/bitcoin/issues/31111)
I've got a Apple M2 Max macbook pro, running sequoia 15.0.1. I'm trying to follow the instructions for fuzzing on macos, but I'm running into trouble.

I reinstalled llvm via `brew install`, as the instructions indicated:

```
$ clang --version
Homebrew clang version 19.1.2
Target: arm64-apple-darwin24.0.0
Thread model: posix
InstalledDir: /opt/homebrew/Cellar/llvm/19.1.2/bin
$ clang++ --version
Homebrew clang version 19.1.2
Target: arm64-apple-darwin24.0.0
Thread model: posix
Ins
...

💬 sipa commented on issue "Trouble fuzzing on macos":
(https://github.com/bitcoin/bitcoin/issues/31111#issuecomment-2421978663)
From https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md#macos-hints-for-libfuzzer:

> $ cmake --preset=libfuzzer \ -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \ -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \ -DAPPEND_LDFLAGS=-Wl,-no_warn_duplicate_libraries

🤔 stickies-v reviewed a pull request: "[refactor] Cleanup BlockAssembler mempool usage"
(https://github.com/bitcoin/bitcoin/pull/28843#pullrequestreview-2377754301)
Approach ACK, code LGTM 60a0f2ac88c57fdf4482dade2ce409ef2da65998 modulo my comment about no-op. I don't feel particularly strong about the current vs the previous approach, but this has the benefit of being a much smaller change, and it works well.

💬 stickies-v commented on pull request "[refactor] Cleanup BlockAssembler mempool usage":
(https://github.com/bitcoin/bitcoin/pull/28843#discussion_r1806299068)
I feel a bit uneasy about adding this no-op, it makes understanding the program flow more difficult and I feel like it is a potential footgun. What do you think about asserting the mempool is configured instead?

<details>
<summary>git diff on 60a0f2ac88</summary>

```diff
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 9e6954d257..afa410da10 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -141,7 +141,9 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNew
...

🤔 tdb3 reviewed a pull request: "tracing: explicitly cast block_connected duration to nanoseconds"
(https://github.com/bitcoin/bitcoin/pull/29877#pullrequestreview-2377938779)
post merge ACK cd0edf26c07c8c615f3ae3ac040c4774dcc8e650

Left on question, but it's minor.

💬 tdb3 commented on pull request "tracing: explicitly cast block_connected duration to nanoseconds":
(https://github.com/bitcoin/bitcoin/pull/29877#discussion_r1806407824)
`In case the duration unit is off, we'll detect it with this assert`

Looks like this detects a change from a less to a more granular duration unit. Would this detect a change from more granular to less granular (e.g. from nanoseconds back to microseconds)? Unlikely to encounter this though, now that the cast to nanoseconds is occurring.

💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1806410808)
I couldn't reproduce the warning locally, but changed it to modify every value in `val.data()` instead to make sure none of them can be optimized away - would you be so kind and check if this solves the undefined behavior?

💬 kevkevinpal commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2422470853)
Concept ACK

👍 tdb3 approved a pull request: "netinfo: add peer services column and outbound-only peers list"
(https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2378098839)
re ACK 17f8f03ec69b6dd3d61137a39b8f88201ec500dc

Code reviewed diff.

Ran sanity check using the new `outonly` arg:
```
$ build/src/bitcoin-cli -signet -netinfo 1
Bitcoin Core client v28.99.0-17f8f03ec69b signet - server 70016/Satoshi:28.99.0/

<-> type net serv v mping ping send recv txn blk hb addrp addrl age id
in npr nwl2 2 1 2 38 24 1 2 6 13
out full ipv6 nwl 1 82 91 24 24 3 . 1001
...

👍 stickies-v approved a pull request: "init: Some small chainstate load improvements"
(https://github.com/bitcoin/bitcoin/pull/31046#pullrequestreview-2377966456)
ACK 31cc5006c3de4dd6a1f7a238684163956604df45

💬 stickies-v commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#discussion_r1806424005)
unrelated nit / probably scope creep but it is a "small chainstate load improvement" so leaving here anyway, can be simplified:

<details>
<summary>git diff on 31cc5006c3</summary>

```diff
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index fa8d7a386f..4837135dee 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -240,11 +240,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
// A reload of the block inde
...

💬 stickies-v commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#discussion_r1806649506)
Hmm, then again, that breaks the pattern here of only (explicitly) returning early for failure, so could also leave as-is.

👍 pinheadmz approved a pull request: "Split CConnman"
(https://github.com/bitcoin/bitcoin/pull/30988#pullrequestreview-2373476515)
ACK 70c2f13f83a5cc740330d0b4af9cbd74515be6b2

Built and ran tests on macos/arm as well as debian/x86.

Synced to signet with this build on a debian server, then synced from that node locally using the macos build.

I left a lot of comments below but most are for myself to track during rebases, indicate to the maintainers the depth that I'm actually reviewing, and also some notes for myself -- I'll be rebaing my non-libevent http server on this banch and consuming the new SockMan.

I have
...

💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1803692315)
41c87ddb3d7a18d2c0fa7eccfbde57e9d6e898c2

Maybe these are expected to be set by callers in future commits, so I'm just leaving myself a note to look for `SO_KEEPALIVE` and `TCP_NODELAY`