👍 hebasto approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707143194)
ACK 0989d00b7b298bef9e1faf04c1753a024b7dfd9e, I've applied backport locally and got a zero diff. Also I've reviewed the last two commits.

👍 TheCharlatan approved a pull request: "[26.x] Backports for rc2"
(https://github.com/bitcoin/bitcoin/pull/28754#pullrequestreview-1707161659)
lgtm ACK 9c29048dac5dfa5a06323b350156b1212b87d56d

💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1787970115)
No idea what is wrong with Microsoft/Azure/GHA again. Locally it works fine, but not on their infra:

https://github.com/bitcoin/bitcoin/actions/runs/6711963492/job/18240509895?pr=28349#step:6:210

```
checking whether clang is Clang... yes
checking whether pthreads work with "-pthread" and "-lpthread"... yes
checking whether Clang needs flag to prevent "argument unused" warning when linking with -pthread... no
checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE
checking
...

💬 theuni commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1787983662)
Concept ACK
[This comment](https://github.com/mesonbuild/meson/issues/10673#issuecomment-1243061073 ) (and the rest of that thread) helped me to understand what was going on here. As I understand it, "-lssp" should've never been needed. It was used as a hack for years, but is no longer required after mingw-w64 v11.

I'm confused like @fanquake though. I would have expected us to need to carry the hack for a while, considering that the fix is from this year.

💬 mzumsande commented on pull request "addrman: log AS only when using asmap":
(https://github.com/bitcoin/bitcoin/pull/28729#issuecomment-1788025388)
Code Review ACK 02a4f1a3859ed7e865641b35ca1bc9ce711e696f

💬 jonatack commented on issue "Evicting and filling attack for linking multiple network addresses":
(https://github.com/bitcoin/bitcoin/issues/28760#issuecomment-1788045181)
Introducing some randomness into the eviction algorithm to make it less deterministic -- instead of, or in addition to, random disconnection delay -- might be a potentially low-hanging countermeasure.

*"Bitcoin allows multiple connections from one single address (Saad et al. [2021](https://cybersecurity.springeropen.com/articles/10.1186/s42400-023-00182-9#ref-CR45)). This property significantly reduces the attack cost."*

#28248 proposes to improve or address this in several ways.

*"We d
...

💬 pablomartin4btc commented on pull request "gui: fix crash on selecting "Mask values" in transaction view":
(https://github.com/bitcoin-core/gui/pull/774#issuecomment-1788048860)
I do agree with @theStack backporting (checked it with the `git tag` command) since the issue technically is the "conflict" between these 2 lines:
https://github.com/bitcoin-core/gui/blob/3c0b66c2ece7695ab59f466d7b4d81a4c18ffd76/src/qt/transactionview.cpp#L532-L533

The last one (#L533) was added by the #708 (merged in v25 - I wanted to say I'm innocent but no haha), so another way to solve this issue is just by removing #L532, which I've tested and works fine.

💬 jonatack commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#issuecomment-1788065706)
Concept ACK.

(Maybe unrelated, but the Win64 CI task is one I've often wished for more debug info from when a unit test fails.)

💬 kevkevinpal commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#discussion_r1378227825)
why do we only have the first few digits here, I tried with `046d6b657901000000` and it worked fine

`assert_equal(all([not line == "046d6b657901000000" for line in f]), True)`

💬 jonatack commented on pull request "build: remove duplicate `-lminiupnpc` linking":
(https://github.com/bitcoin/bitcoin/pull/28755#issuecomment-1788174027)
ACK b74e449ffa9965f18037f0322ea178e2fe1dbc18

Build output diff is the same as my previous https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1705089533, and with the latest push also see a similar improvement as reported by @hebasto in https://github.com/bitcoin/bitcoin/pull/28755#pullrequestreview-1706070279:

master
```
checking for miniupnpc/miniupnpc.h... yes
checking for upnpDiscover in -lminiupnpc... yes
checking for miniupnpc/upnpcommands.h... yes
checking for upnpD
...

💬 ajtowns commented on pull request "refactors for subpackage evaluation":
(https://github.com/bitcoin/bitcoin/pull/28758#discussion_r1378307864)
Could alternatively make these member functions of `Package` ?

👍 pablomartin4btc approved a pull request: "[do not merge] validation: assumeutxo params mainnet"
(https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-1707472149)
tACK 0800be24bc6de4154653be75b8fa9902a4a8c3a7

```
./contrib/devtools/utxo_snapshot.sh 800000 - ./src/bitcoin-cli -datadir=${AU_DATADIR}
Rewinding chain back to height 800000 (by invalidating 00000000000000000000e26b239cf19ec7ace5edd9694d51a3f6933247720947); this may take a while
Generating txoutset info...
{
"height": 800000,
"bestblock": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054",
"txouts": 111535121,
"bogosize": 8443325590,
"hash_serialized_3": "6e
...

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1788285499)
I would expect that a wallet that connects to us over (untrusted) p2p will get the exact same info regardless of background validation progress.

💬 Sjors commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1788287616)
@pablomartin4btc thanks for checking that you get the same snapshot. In addition it would be good to test if you're able to load the snapshot and complete the background sync.

💬 achow101 commented on pull request "wallet: Cleanup accidental encryption keys in watchonly wallets":
(https://github.com/bitcoin/bitcoin/pull/28724#discussion_r1378328199)
This prefix matches all possible `mkey` records so it can make sure that there are no `mkey`s slipping in elsewhere.

💬 pablomartin4btc commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-1788298888)
> @pablomartin4btc thanks for checking that you get the same snapshot. In addition it would be good to test if you're able to load the snapshot and complete the background sync.

I've already checked IBD completed fine with`[background validation]`finished, but with the previous`hash_serialised`, as I wanted to verify that #28698 was only happening before the IBD completion.

I'll run all the process again with the new`hash_serialised`value.

💬 toolsopen commented on issue "sendrawtransaction takes too long":
(https://github.com/bitcoin/bitcoin/issues/28745#issuecomment-1788341767)
https://github.com/bitcoin/bitcoin/issues/12115
<img width="921" alt="image" src="https://github.com/bitcoin/bitcoin/assets/21998212/eb9c1ef3-bb33-4600-a5d7-f9fd0dcddfbe">

@achow101 @maflcko @willcl-ark @gregwebs I saw that this issue was raised in 2018, but no attention was paid to it

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378361925)
I think the usage of virtual size to build the ancestor score of a package might come with incentive suboptimal block template if you have a significant number of segwit spends constituting it (sounds 90% as of 2023). A 500 vbyte transaction A paying 100 sats with 100 vbytes of witnesses isn’t equivalent to a 500 vbyte transaction B paying 100 sats with 20 vbytes of witnesses considering block max size is checked against `MAX_BLOCK_WEIGHT` in `CheckBlock()`.

🤔 ariard reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1707520236)
Review at commit “[txpackages] add AncestorPackage for linearizing packages”.

💬 ariard commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1378371630)
If my understanding of the guarantees of the `PackageEntry` comparison operator is correct, a breakage of the guarantee would be a yield `AncestorPackage` that produce a consensus-invalid block template, e.g a more incentive-compatible subset of descendants where the parents are sorted after. This comment to understand how test coverage should be built for this PR.