💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158456950)
yep thanks, fixed

👍 hebasto approved a pull request: "ci: fix git dubious permissions error"
(https://github.com/bitcoin/bitcoin/pull/27423)
re-ACK ed4a8339b8fe796b4668e206d7fb9c2b120f8eb2

💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1497434168)
Rebased past #27406.

Note that the branch protection option being added to libevent here, can now exist inside the `NO_HARDEN` clause.

🚀 fanquake merged a pull request: "ci: use clang-16 in tidy task"
(https://github.com/bitcoin/bitcoin/pull/27404)

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158537830)
`--no-trust-builder-keys` is no-longer a thing? Arguments here need re-ordering as well.

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158526427)
Can drop `VERSIONPREFIX` from this output.

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158530776)
note that the keyserver we currently "recommend" is `hkps://keys.openpgp.org`. See #22688, #23466.

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158540308)
```suggestion
--min-good-sigs 10
```

💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1497547148)
Rebased, and added a number of small doc fixups.

💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1158582475)
I think that is a fair point. The issue is that if we ever need to upgrade libevent due to security vulnerability it will break and you'll have to figure this out again and change `build_msvc/bitcoin_config.h.in`. This addresses that issue today rather then at some point in the future. That said, if we expect to move to CMake soon, then it makes sense to move to the previous version and get the minimum diff.

I'll change this to `2.1.12#7`, but I'm open to changing to back to `2.1.12+20230128`
...

📝 fanquake converted_to_draft a pull request: "build: LLVM 16 & LLD based macOS toolchain"
(https://github.com/bitcoin/bitcoin/pull/21778)
This switches us to using a [LLVM](https://llvm.org/) & [LLD](https://lld.llvm.org/) based toolchain for macOS builds. Much work has recently been done on a [new Mach-O backend](https://reviews.llvm.org/rG03f43b3aca3) for LLD and as of LLVM 13 it has become [the default backend](https://reviews.llvm.org/D95204).

With this change I'm able to everything compile and link `bitcoind` and utilities. `bitcoin-qt` and `libbitcoinconsensus` don't currently build. The binaries produced run OK when tak
...

💬 jonatack commented on pull request "test: create random and coins utils, add amount helper, dedupe add_coin":
(https://github.com/bitcoin/bitcoin/pull/26940#issuecomment-1497607769)
undefined@glozow Thanks for bringing it up! The rationale was in this thread: https://github.com/bitcoin/bitcoin/pull/26940#discussion_r1083532109. I noticed this as well, and that `src/test/util/chainstate.h` has the same dependency, and made a patch to remove those. Let me see if I can dig it up.

👍 pablomartin4btc approved a pull request: "wallet: finish addressbook encapsulation"
(https://github.com/bitcoin/bitcoin/pull/26836)
undefinedtested ACK.
Performed manual testing using `bitcoin-qt`.

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1497743020)
undefinedThanks for the review @fanquake, comments addressed.

LMK if you'd like me to squash. I could either squash mine into the original authors' or mine all into one "fixup" on top of theirs.

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1497746671)
undefinedrebased on master and addressed some nits from follow-up PR https://github.com/bitcoin-core/gui/pull/723

💬 fanquake commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1497754148)
undefinedRetesting this on master (04595484d97100a50c146e5fc080319d9e0f5ca4). 1 line missing from the above output, which is dumped out in the test failure output before the test is re-run is `libc++abi: Pure virtual function called!`:
```bash
2023-04-05T15:47:47.686929Z (mocktime: 2020-08-31T15:34:12Z) [basic block filter index] [util/thread.cpp:20] [TraceThread] basic block filter index thread start
test/blockfilter_index_tests.cpp(147): fatal error: in "blockfilter_index_tests/blockfilter_index_ini
...

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1497757188)
undefined@theuni I don't particularly mind. I think the commits here look ok as-is. If you want to squash them all down into a fixup, that's probably also ok, but going to be a bit messy. Squashing all the changes back into the original commits is probably going to be more messy again.

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1497786231)
undefined> We can clean up the documentation and/or behavior of -logips in another PR.

For sure, I can work on that after this one.

💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1497789875)
undefinedPushed this on my lunch break. With @hebasto's help this is now a single file, 8 line change. =)

💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1158760104)
undefinedWith `2.1.12#7` we don't need to include `bcrypt.lib` as an additional dependency and so I have removed it. If we upgrade libevent we will need to add this.