⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34068)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34069)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34070)

⚠️ sajadvilsin-cmyk opened an issue: "ok"
(https://github.com/bitcoin/bitcoin/issues/34071)

📝 sajadvilsin-cmyk opened a pull request: "Create sajad"
(https://github.com/bitcoin/bitcoin/pull/34072)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 ajtowns commented on pull request "refactor: Use NodeClock::time_point for m_addr_token_timestamp":
(https://github.com/bitcoin/bitcoin/pull/34059#issuecomment-3648529415)
What do you think of getting all the `peer->foo` to `peer.foo` noise done first, rather than doing it bit by bit? Here's a scripted-diff approach that might be easy to review: https://github.com/ajtowns/bitcoin/commits/202512-peer-by-ref/

💬 ajtowns commented on pull request "logging: API improvements":
(https://github.com/bitcoin/bitcoin/pull/34038#discussion_r2615873425)
Changed it to just support both include or debug as aliases indefinitely. (Can remove the `include=` alias when the deprecatedrpc option is removed)

I don't think it makes sense to ship a bitcoind/bitcoin-cli pair that don't work together under some configuration param (ie `bitcoin-cli -named include="['net']"` doesn't work because cli won't parse it as json, and `debug=["net"]` won't work because you're running with `deprecatedrpc=logging`)

💬 ajtowns commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3648572091)
> it seems a bit absurd to check every `50ms` for a timeout of 20 minutes. Even the initial waiting time `m_peer_connect_timeout` from `ShouldRunInactivityChecks()` for a new peer doesn't really be checked _that_ frequently.

I think if you're running a listening node with many real inbound peers, on mainnet where there seems to be maybe 4.5 tx/s, you probably expect to receive socket data from each peer every 2 seconds or so (announcing the ~9 txs that have been received in that time). With 8
...

💬 arejula27 commented on pull request "refactor: Add util::Result failure types and ability to merge result values":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-3648673018)
> TBH I don't know of code that would directly benefit from combining ErrorString with std::expected because I think validation and wallet code are better off using util::Result

If you don't see any benefits keep your implementation, a generalisation can be implemented later if required 😄

💬 ajtowns commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2616011262)
Should these log messages include "I2P" somewhere? Otherwise figuring out what SAM relates to may be confusing. The help text for `-i2psam` describes it as an "I2P SAM proxy".

💬 Ataraxia009 commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2616086274)
doc string can be changed. No risk to keep it but also no point to add it imo. Can just be a normal function that only rate limits >= Info Level and only used within this file.

But just move forward its fine

💬 w0xlt commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2616151939)
> can this result in an infinite loop or disconnect/reconnects?

From what I understand, yes. That's the point.

💬 maflcko commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#discussion_r2616285391)
I may be missing something obvious. https://godbolt.org/z/8GM97aYhs seems to compile fine. And https://eel.is/c++draft/time.duration.cons doesn't mention `noexcept`. Can you give a minimal reproducer, or link to the std docs?

💬 yuvicc commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3649377448)
ACK 8ca9997e48bb2067ea83fabb1c640af72178f97c

💬 maflcko commented on pull request "Make `transaction_indentifier` hex string constructor evaluated at comptime":
(https://github.com/bitcoin/bitcoin/pull/34063#discussion_r2616304495)
> Had a similar attempt in [#31991 (comment)](https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1995691590) - what do you think @maflcko?

Either should be fine. I just think there is no need to provide the exact same functionality via two ways. That seems confusing.

💬 stickies-v commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#issuecomment-3649442978)
re-ACK fa8a5d215c5a81a7282fd5dd1098f9d3fa40e5db

👍 pablomartin4btc approved a pull request: "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3574392235)
re-ACK b3046cca7182f3399a221757318d24e203092301

💬 maflcko commented on pull request "log: Remove brittle and confusing LogPrintLevel":
(https://github.com/bitcoin/bitcoin/pull/34051#discussion_r2616320977)
thx, done in all commits

👍 pablomartin4btc approved a pull request: "log: Remove brittle and confusing LogPrintLevel"
(https://github.com/bitcoin/bitcoin/pull/34051#pullrequestreview-3574394123)
re-ACK fa8a5d215c5a81a7282fd5dd1098f9d3fa40e5db

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2616332760)
I don't think so - the function does not read it, only sets it. However, when the proxy is not needed, the function would leave it untouched. The only caller passes empty optional, so it is fine, but I changed it to always set it from inside the function - either to empty optional (if proxy is not needed) or to a proxy.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2616334277)
There is a sleep of 500ms just below the log.