💬 apoelstra commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127508227)
@luke-jr are you suggesting that all consumers should call the entire RPC surface to confirm which calls are present, what the types of all inputs are, what the types of all outputs are, etc., in order to determine whether their software is compatible with the interface?

👍 stickies-v approved a pull request: "[26.x] archive 26.1 release notes + backports"
(https://github.com/bitcoin/bitcoin/pull/29899#pullrequestreview-2074368997)
ACK a29e9e5fea98925cd19c4e9e8738de67d04da549 modulo release notes touchup

All backport commits are clean, except for:
- b1481467bdf6c1300341d1e2ca399ca5e30fdace: because of https://github.com/bitcoin/bitcoin/pull/28955
- 109b9d74f50651aa4765d6eec047f09e497215fd: because of LLVM version bump in #29659
- 7d13e6ab51a02db33fea4df61f70c3f919016a8b: because of removing -rpcserialversion (#28890, and #28438)

All of these look good to me.

nit: in b1481467bdf6c1300341d1e2ca399ca5e30fdace comm
...

💬 stickies-v commented on pull request "[26.x] archive 26.1 release notes + backports":
(https://github.com/bitcoin/bitcoin/pull/29899#discussion_r1611933793)
I think we usually keep this in for backports too?

💬 furszy commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1611957557)
thinking more about this, let's remove this timeout=3. It could fail on slow system or when running valgrind.

💬 maflcko commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1611961965)
Any reason to put the stop under the busy loop here?

💬 maflcko commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1611961304)
any reason to use the busy loop here?

💬 maflcko commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1611962533)
nit: Should use named args for integral literal arguments

💬 glozow commented on pull request "[26.x] archive 26.1 release notes + backports":
(https://github.com/bitcoin/bitcoin/pull/29899#issuecomment-2127527079)
Thanks so much @stickies-v! Nice catch, fixed the transifex line and PR number

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611967800)
And maybe:

```cpp
// rt_msghdr is followed by zero or more sockaddrs, as indicated by rtm_addrs
auto sa = (const struct sockaddr*)(p + sizeof(rt_msghdr));
```

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611968560)
How about:

```cpp
// rt_msghdr is followed by zero or more sockaddrs, as indicated by rtm_addrs
auto sa = (const struct sockaddr*)(p + sizeof(rt_msghdr));
```

💬 stickies-v commented on pull request "[26.x] archive 26.1 release notes + backports":
(https://github.com/bitcoin/bitcoin/pull/29899#issuecomment-2127533094)
re-ACK aa7e876953c460e8ff97a719fdb18f4f3ac4896f, only changes are fixing commit msg and transifex reference

💬 luke-jr commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127533517)
Consumers don't need the entire RPC surface, only specific parts. If a compatibility check is desired, that's indeed how it should be (and typically is) done - just like autotools/cmake checks for APIs. It could also be done at runtime in many cases: if a call fails, fallback to another one (and maybe flag so you don't try the better call next time).

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2127536164)
sadly, forced to rebased post https://github.com/bitcoin/bitcoin/pull/29873

Should be pretty trivial to review with something like:
`git range-diff master 63cfa4c 2fd34ba504957331f5a08614b6e1f8317260f04d`

✅ achow101 closed an issue: "Use AppData\Local instead of AppData\Roaming"
(https://github.com/bitcoin/bitcoin/issues/2391)

🚀 achow101 merged a pull request: "system: use %LOCALAPPDATA% as default datadir on windows"
(https://github.com/bitcoin/bitcoin/pull/27064)

💬 maflcko commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2127543290)
In theory the consumer could call the `help` RPC (or a computer readable help, once there is one, see https://github.com/bitcoin/bitcoin/issues/29912) and use that to double check that the views match. However, this may be more work for consumers to implement, as opposed to a simple version check.

Concept ACK

💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2127548244)
reACK 2fd34ba504957331f5a08614b6e1f8317260f04d via range-diff

💬 achow101 commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2127566569)
ACK 542e13b2937356810bda2c41be83c3b1675e2f2f

💬 maflcko commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2127569909)
> I don't see where I would get a chunk of time to do that right now....

Sure, no rush. I'll probably take some time to pin this down. (I don't have an AWS account, so I can't test it, but maybe someone else has).

Some other ideas to test in the meantime:

* Try another filesystem instead of xfs
* Try the master branch (not for production, just for testing whether the issue still happens there)

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1612003469)
Right, let's add `sizeof(rt_msghdr)` manually instead of doing the +1 trick.

> That makes sense. Whereas rt_msghdr is fixed length I guess, so you're able to use rt + 1 above (I'm still a bit terified that compilers don't care).

If we can hardcode the assumption that `sa_len` is the first byte (which it is) then we can avoid casting and dereferencing sockaddr, anywhere.