👍 alfonsoromanz approved a pull request: "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection"
(https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2008812232)
Tested ACK d3da5025f61534d126fef12f198afb65f3a17897

💬 fanquake commented on pull request "test: Add large aligned vmov check for mingw":
(https://github.com/bitcoin/bitcoin/pull/29874#issuecomment-2063806688)
Looks like `python-capstone` at least exists in Guix already: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/engineering.scm#n1820.

💬 laanwj commented on pull request "test: Add large aligned vmov check for mingw":
(https://github.com/bitcoin/bitcoin/pull/29874#issuecomment-2063819614)
> Looks like python-capstone at least exists in Guix already:
> https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/engineering.scm#n1820.

That's good to know! i'm not that surprised, it's a very popular library for doing binary analysis and reverse engineering kind of things.
i'll have a look at adding it.

📝 fjahr opened a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904)
Removing dependencies is a general goal of the project and the xz backdoor has been an additional wake up call recently. Libevent shows many of the same symptoms, few maintainers and slow releases. While libevent can not be removed completely over night we should start removing it’s usage where it's possible, ideally with the end goal to removing it completely.

This is a pretty easy win in that direction. The [`evhttp_uridecode` function from libevent](https://github.com/libevent/libevent/blo
...

💬 laanwj commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2063835209)
Concept ACK
yes please... the hooking we do for `URL_DECODE` and `UrlDecodeFn` made me kind of scared, to be honest (in that context), i think can we get rid of that at the same time?

This will also fix @luke-jr's #29654.

💬 furszy commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570731552)
In 4b05c654c1:

Using `m_best_header` shouldn't be working (at least not as you expect) for `invalidateblock`.

During `InvalidateBlock()`, we first disconnect all blocks, then call `CheckBlockIndex` without updating `m_best_header` (this is done later on the flow, when we call `InvalidChainFound()`). So, by the time `CheckBlockIndex` is called, `best_hdr_chain` is loaded with a chain that is not the active one.

Any reason not to use `ActiveTip()` instead of `m_best_header` here?

💬 furszy commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#issuecomment-2063852471)
@DrahtBot, I just reviewed it..

💬 willcl-ark commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063858757)
Ok there seems to be some push back here. Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into `/usr/local` wouldn't have random odds-and-ends ending up in the root there, but happy to conceed on this (perhaps I install binary tarballs [weirdly](https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1895518581)?)

Seems like the path of least resistance is just to append `.example`, so I've done that here now.

> I think all we
...

💬 TheCharlatan commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2063858822)
Concept ACK

💬 ryanofsky commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570756697)
> Any reason not to use `ActiveTip()` instead of `m_best_header` here?

I think the reason is just that we want `best_hdr_chain` to be the longest chain with the most forks for the optimization to work well. We don't actually care which chain it is because CheckBlockIndex will traverse all the blocks in any case. If a different chain is chosen it just means the blocks will be traversed in a different order, less or more efficiently.

💬 maflcko commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#discussion_r1570777163)
> EDIT: wondering why there are limits

There needs to be a limit, because each test gets allocated the full (maximum) range of ports for the duration of the test_runner (that is for the duration of *all* tests, not the test itself) to avoid port collisions. Thus, the limit needs to exist, because there is a finite number of ports on any system.

👍 vasild approved a pull request: "net: attempts to connect to all resolved addresses when connecting to a node"
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-2008933057)
ACK ae7afa5547889ccf04c1522555e28e025d14f335

💬 vasild commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1570773337)
nit: since this line is being changed anyway, consider this:

```suggestion
LogPrintLevel(BCLog::PROXY, BCLog::Level::Debug, "Using proxy: %s to connect to %s\n", proxy.ToString(), target_addr.ToStringAddrPort());
```

This would take care to put IPv6 addresses in `[` `]` instead of printing something like `2001:470:88ff:2e::1:8333`

💬 furszy commented on pull request "validation: improve performance of CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/28339#discussion_r1570807877)
ok, yeah. Then it is not an issue but, isn't error-prone to call `best_hdr_chain` something that might not be the best chain then? E.g. it seems that if we ever add a check only for the active chain blocks here, the `best_hdr_chain.Contains()` usage temptation will be there.

💬 stickies-v commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#issuecomment-2063926648)
Concept ACK

💬 brunoerg commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1570815195)
```suggestion
(void)crypt.Decrypt(cipher_text_ed, plain_text_ed);

```

💬 brunoerg commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1570821642)
Since you're using `ConsumeDeserializable`, I do believe you should adopt the approach introduced in "fuzz: Avoid timeout and bloat in fuzz targets" fabb5046a7365af3079e6e45606d63576bc6ad12

💬 achow101 commented on pull request "doc: update release-process.md":
(https://github.com/bitcoin/bitcoin/pull/29645#issuecomment-2063938314)
I think the "Update repositories and websites for new version" section could be split into 2 distinct steps of update the website, then update the packaging repos. Apparently flathub polls the website for new releases and their bot will automatically open a pr to the flathub repo with the new version hashes. So that step must be done after the website is updated.

💬 josibake commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063942430)
> Personally, I'd prefer to see files neatly in directories so that folks installing from a tarball directly into /usr/local wouldn't have random odds-and-ends ending up in the root there

We still have the `README.md` in the root, so moving this file doesn't really address that concern. If there is a compelling argument for having _nothing_ in the root, then I'd argue `config/bitcoin.conf.example` would make the most sense (as this is what I've seen for most other tarball distributed software
...

💬 josibake commented on pull request "Move bitcoin.conf to /share/examples dir in releases":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063950133)
LGTM, although the commit message and PR title still mention moving to `share/examples`