💬 Sjors commented on pull request "wallet: Fix migration of wallets with txs that have both spendable and watchonly outputs":
(https://github.com/bitcoin/bitcoin/pull/28868#issuecomment-1809666676)
Other than that a47f220933f202dd1f972d30f6cb31b09ca0a225 works. Without it, when migrating a watch-only wallet it rescans from the wallet birth height. With it, it does not rescan.

Regarding 65b2da99fe18768d1bd4cd67522f7031f16a3420 this gets a bit unwieldy because there are many places the migration can fail. Maybe just let the caller `MigrateLegacyToDescriptor` attempt a reload if migration failed.

💬 hebasto commented on issue "macOS qt QTimer::stop crash on v26.0rc2":
(https://github.com/bitcoin/bitcoin/issues/28867#issuecomment-1809792317)
> I believe I was closing the application (according to the stacktrace)

Is it reproducible?

💬 willcl-ark commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1809819025)
# V26.0 Testing Guide notes

Hey @m3dwards thanks for taking this on, the guide looks great so far! I didn't practically run through all the tests myself yet, but left a few tiny nits from a read-through below:

## Compile Release Candidate

> it is recommended that you compile with sqlite(--with-sqlite=yes), so make sure you have installed the sqlite3 dependency.

default is `--with-sqlite=auto` which will pick up `sqlite3` if it's installed, so this isn't particularly required so long
...

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#discussion_r1392260431)
I'll add description to description fields later.

🚀 fanquake merged a pull request: "depends: latest config.guess & config.sub"
(https://github.com/bitcoin/bitcoin/pull/28781)

💬 Sjors commented on pull request "guix: update signapple (drop macho & altgraph)":
(https://github.com/bitcoin/bitcoin/pull/28859#issuecomment-1809855632)
ACK f718a74b124c723548f5d1961ef4e3aa15c33847

Guix hashes for 'x86_64-apple-darwin` and `arm64-apple-darwin' (generated on x86_64 linux):

```
7ea71c9d553c8afaa6c1f8ba72b643783910a972faabd6712f85020dbe8dbdae guix-build-f718a74b124c/output/arm64-apple-darwin/SHA256SUMS.part
3263327cbd4ee22c21256410da377b38399d07f163a6d1c6c266aa1f9024de64 guix-build-f718a74b124c/output/arm64-apple-darwin/bitcoin-f718a74b124c-arm64-apple-darwin-unsigned.tar.gz
dc639d8aff10827c0e1a24e4edf16425cf08d443948659
...

👍 vasild approved a pull request: "p2p: make block download logic aware of limited peers threshold"
(https://github.com/bitcoin/bitcoin/pull/28120#pullrequestreview-1729314172)
ACK a0687ffd568b0984a8b1e51fd0061f87f25e95d7

Thanks!

💬 bufo24 commented on pull request "log: torcontrol opt checks":
(https://github.com/bitcoin/bitcoin/pull/28780#issuecomment-1809867084)
> Are you still working on this?

no unfortunately not

✅ fanquake closed a pull request: "log: torcontrol opt checks"
(https://github.com/bitcoin/bitcoin/pull/28780)

🚀 fanquake merged a pull request: "build: remove `-bind_at_load` usage"
(https://github.com/bitcoin/bitcoin/pull/28783)

👍 vasild approved a pull request: "net: support unix domain sockets for -proxy and -onion"
(https://github.com/bitcoin/bitcoin/pull/27375#pullrequestreview-1729334104)
ACK 6dddc1c2eda514d35219cce03c229bd6f822be55

💬 dergoegge commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1809928608)
Should we do the same for `make cov_fuzz`? i.e. separate running the fuzz test runner from generating the coverage report.

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1809939629)
It would be useful to add a `mempool_backwards_compatibility.py` test to illustrate how the new rules interact with older nodes. It could have two modern nodes and one v25 (or v26) node. Some of the tests you deleted in this branch could be moved there. E.g. the test could demonstrate how RBF rule 2 is not enforced when relaying to the new node, but it is when relaying to the v25 node.

Benchmarks on a 2019 MacBook Pro (2,3 GHz 8-Core Intel Core i9), plugged in:

```
% src/bench/bench_bitco
...

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392325037)
dd6684ab665bc8ae76b76fdd2e578fc77d562a52: While you're touching this, can you rename `MempoolCheck` to `MemPoolCheck`, `MempoolEviction` to `MemPoolEviction` and `ComplexMemPool` to `MempoolComplex`? That makes `-filter=MemPool.*` work

As a workaround, `-filter=.*Mem.*` does work.

💬 maflcko commented on pull request "test: Generate coverage report without running tests":
(https://github.com/bitcoin/bitcoin/pull/28772#issuecomment-1809957950)
> I don't think so? (besides renaming, if we do that). The way I read the docs was that `make check` had to be run first, and separately. I hadn't expected `make cov` to run them for me.

The docs don't mention `make check` and the snippet similar to the docs may also used by external scripts. For example, https://github.com/maflcko/b-c-cov/blob/83bc0912e2a33cfac2195645ab3a12d092fa7ba3/.cirrus.yml#L46-L47

💬 maflcko commented on pull request "tests: Fix LCOV_OPTS to be in the correct position":
(https://github.com/bitcoin/bitcoin/pull/28771#issuecomment-1809977209)
I agree that it is wrong. I mostly wonder why it was working fine previously, when it shouldn't.

💬 fjahr commented on pull request "test: add stress test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1809988538)
I would like it more if you keep the `asmap.py` in seeds because ultimately I want to move it from there to `contrib/asmap` in #28793 and I think the primary function of that code is as a tool, not as a test lib.

And if this is not being run as part of the functional test suite (which I only understood just now), then I am not sure it belongs there. Maybe there is precedent for something similar but to me it feels more like this is something comparable to `test_utxo_snapshots.sh`? I think the
...

💬 maflcko commented on pull request "[WIP] test: migrate to some per-symbol ubsan suppressions":
(https://github.com/bitcoin/bitcoin/pull/28865#issuecomment-1809989839)
Should be fine, if CI passes. Can easily be reverted, if it turns out failing locally.

💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1809997651)
> It's pretty misleading though, since `CScriptNum` is little-endian: if you see `0x0100` you would expect that to be 256, but for us it would actually be non-minimally-encoded 1; For us, `0x0201` would actually be less than `0x0102`, etc.

Those smaller numbers will be shown in decimal still. For larger pushdata, hex is already shown in the little-endian encoded byte order. If `0x` is too suggestive of a big-endian hex number (a fair point), then maybe `bytes(123456789abcdef)` is clearer and
...

📝 BrandonOdiwuor opened a pull request: "GUI getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777)
Fixes https://github.com/bitcoin-core/gui/issues/645.
Add `getrawtransaction` RPC to GUI on `Help -> Verify external txid.` tab


verbose off: `getrawtransaction txid false blockhash`

verbose on: `getrawtransaction txid true blockha
...