💬 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.

✅ achow101 closed an issue: "Possible savings in `dumptxoutset` serialization format (~20%)"
(https://github.com/bitcoin/bitcoin/issues/25675)

🚀 achow101 merged a pull request: "rpc: Optimize serialization and enhance metadata of dumptxoutset output"
(https://github.com/bitcoin/bitcoin/pull/29612)

💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1612008655)
I agree. I will address it.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1612015499)
Currently i have the following, having changed the pointer arithmetic to byte offsets:
```c++
// Iterate over messages (each message is a routing table entry).
for (size_t msgptr = 0; msgptr < buf.size(); ) {
const struct rt_msghdr* rt = (const struct rt_msghdr*)(buf.data() + msgptr);
// Iterate over addresses within entry, get destination and gateway (if present).
// Pointer to address data within message, starts after header.
size_t saptr = msgptr
...

💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1612017998)
fixed, thanks