💬 ryanofsky commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1664025326)
re: https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663631973

> It is not possible to pass a raw c-string pointer to transifex for translation. Only string literals can be translated.

In case it's not clear, the fix for this problem and the previous [unnecessary translation](https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1663611194) problem is probably to replace `_()` with `Untranslated()` everywhere where in this PR when converting strings to bilingual_str. If we ad
...

💬 hebasto commented on pull request "util: Catch translation string errors at compile time":
(https://github.com/bitcoin/bitcoin/pull/30383#issuecomment-2205838536)
Concept ACK on the PR's goal.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664026678)
No, this doesn't implement the exact retry logic, it seems excessively complicated for communicating with a local router, and re-using the same code for NAT-PMP and PCP keeps the code straighforward.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664031826)
This is intentional. For IPv4 we provide the all-zeroes address, because the router will use its own (NAT), for IPv6 we *want* the external address to be the same as the local address (NAT is not a thing, we want pinholing).

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664034915)
As we receive the UDP packet at all, i think that should be correct. But sure...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664036768)
Again, i intentionally use a simple imlementation here. i don't want to complicate the code too much. As i understand, this covers the practical cases. It chooses the minimum because we don't want holes in coverage while renewing the mapping more than necessary doesn't hurt.

🤔 glozow reviewed a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2156187877)
ACK c1d6e525131cb9e54bbedb79ea64e2469a77aed8

💬 glozow commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1664034005)
Question tangential to this PR: This seems to mean the script execution cache needs something to synchronize access, and it is currently implicitly using `cs_main`? I don't see any threads other than message handler that access it though.

💬 glozow commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1664039214)
c1d6e52513

nit: could do a `static_assert(DEFAULT_VALIDATION_CACHE_BYTES == DEFAULT_SIGNATURE_CACHE_BYTES + DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES)`. Should still hold if allocation of `-maxsigcachesize` changes from 50/50.

💬 maflcko commented on issue "I want to set up a node quickly, do you have a snapshot？":
(https://github.com/bitcoin/bitcoin/issues/30382#issuecomment-2205895693)
> There are some **unofficial, unrelated to this project** assumeutxo snapshots

assumeutxo is a test-only feature for now, not enabled on mainnet, so I don't think this can help here at this point in time. I haven't checked what the site offers, but downloading a datadir from an untrusted source is dangerous and can lead to loss-of-funds, or any other type of bugs.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664067870)
Yes, that makes sense, in the case of onlynet=ipv6 we wouldn't want to advertise a port on ipv4 and vice versa.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664071010)
i've thought about this, but due to how bitcoin handles non-standard ports, we really want the configured external port, not another one, i think it makes sense to keep trying to get it, even if the router assigns us a different one once.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664074217)
> Why make mappings for all local IPv6 addresses?

It's the sensible choice IMO. Some routers will assign a whole bunch of routable IPv6 addresses but only allow pinholing for some. It's doesn't hurt to try.

> Also, it just occurred to me, GetLocalAddresses() operates regardless of -bind= option

Yes. But mind that `-natpmp` and `-pnp` are set to 0 if `-bind` is specified. In that case we assume the user has a more complex configuration and will just do their own mapping.

✅ maflcko closed an issue: "Memory leak loading legacy wallet (BDB 4.8.30)"
(https://github.com/bitcoin/bitcoin/issues/27283)

💬 maflcko commented on issue "Memory leak loading legacy wallet (BDB 4.8.30)":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-2205913297)
Many workarounds have been mentioned in the discussion above. For example:

* disable privdb temporarily
* upgrade bdb
* suppress the warning
* remove bdb

Closing for now. I am happy to answer questions how to implement the workarounds, but I don't think there is anything that can be done in this repo that hasn't already an open pull request. If a pull request is missing, please create one, or explain exactly what should be implemented.

✅ maflcko closed an issue: "Small memory leak when BerkeleyEnvironment::Open fails"
(https://github.com/bitcoin/bitcoin/issues/19034)

💬 maflcko commented on issue "Small memory leak when BerkeleyEnvironment::Open fails":
(https://github.com/bitcoin/bitcoin/issues/19034#issuecomment-2205913896)
Closing per https://github.com/bitcoin/bitcoin/pull/20974#issuecomment-1732362629

💬 TheCharlatan commented on pull request "RFC: Instanced logs":
(https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2205918461)
Re https://github.com/bitcoin/bitcoin/pull/30338#issuecomment-2197683031

> Have a look at https://github.com/ajtowns/bitcoin/pull/9 and see what you think;

This looks good to me, thank you for implementing. Regarding disabling the buffering, I'm not sure if defaulting to off in our code is a good idea. How about this as an alternative: https://github.com/TheCharlatan/bitcoin/commit/86fed95f15fa6cee2e28679fe1ac8c537c4555a0 ? The library would always be configured to not buffer. The user co
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664088207)
Yes, i initially had the wrong idea of what it did, should update the comment.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1664095090)
NATPMP is IPv4 only. In the case of IPv4 we don't need to explicitly bind. We don't do this for PCP either (hence passing the BIND_ANY address).

💬 brunoerg commented on pull request "test: fix debug log assertion in p2p_i2p_ports test":
(https://github.com/bitcoin/bitcoin/pull/30345#issuecomment-2205961761)
> Ok, in the case with python3 -m http.server 60000 the reply is <!DOCTYPE HTML>. In the CI the reply is HELLO VERSION MIN=3.1 MAX=3.1, same as the request.

Yes, just did that to exemplify.

> I wonder what is a sure way to find an address and port that will fail the connection quickly? Maybe pick one of the "reserved" ports from https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers, e.g. 127.0.0.1:250? Or use the P2P port of node0 (p2p_port(0)), bitcoind is listening on it, but a
...