💬 JBaczuk commented on issue "Create a list of validation errors in one location":
(https://github.com/bitcoin/bitcoin/issues/15356#issuecomment-1525876920)
@pinheadmz Probably not at this point.

💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1525910357)
Reviewers, i've seen the comments i'm going to address them soon and rebase this.

I have discussed this with @sipa in person today. A couple notes from my recollection:
- I introduce the `diff` (net difference in stack size before and after executing a fragment) and `exec` (maximum stack size reached during execution) here to account for the maximum stack size at all time. Pieter noticed we may be able to express the size in function of the diff. Or vice versa.
- We tried to assert this pro
...

💬 MarcoFalke commented on issue "Create a list of validation errors in one location":
(https://github.com/bitcoin/bitcoin/issues/15356#issuecomment-1525988287)
Not sure what exactly should be done here. Maybe the error messages can be improved for some, but generally they should be self-explanatory. Recall that the ones you mention are accompanied by an extended description and more specific error message. For example, `"non-mandatory-script-verify-flag (Negative locktime)"`.

💬 MarcoFalke commented on issue "Create a list of validation errors in one location":
(https://github.com/bitcoin/bitcoin/issues/15356#issuecomment-1525988708)
The feature request didn't seem to attract much attention in the past. Thus, closing due to lack of interest, progress and direction.

Pull requests with improvements are always welcome. Moreover, it is possible to re-open this issue or create a new issue referencing it, if there is fresh interest.

✅ MarcoFalke closed an issue: "Create a list of validation errors in one location"
(https://github.com/bitcoin/bitcoin/issues/15356)

💬 brunoerg commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1526013801)
Concept ACK

💬 MarcoFalke commented on issue "depends: Multiple `FALLBACK_DOWNLOAD_PATH`s":
(https://github.com/bitcoin/bitcoin/issues/17234#issuecomment-1526026245)
For reference, for now I am using this diff locally to get the depends from a ipv6 (cloudflare):

```diff
diff --git a/depends/Makefile b/depends/Makefile
index 27bf804..768db6b 100644
--- a/depends/Makefile
+++ b/depends/Makefile
@@ -45,7 +45,7 @@ NO_USDT ?=
NO_NATPMP ?=
MULTIPROCESS ?=
LTO ?=
-FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
+FALLBACK_DOWNLOAD_PATH ?= https://drahtbot.space/depends_download_fallback

C_STANDARD ?= c11
CXX_STANDARD ?= c++17
...

💬 MarcoFalke commented on issue "depends: Multiple `FALLBACK_DOWNLOAD_PATH`s":
(https://github.com/bitcoin/bitcoin/issues/17234#issuecomment-1526028066)
Same as https://github.com/bitcoin/bitcoin/pull/17704

💬 MarcoFalke commented on issue "v 0.17.0 win 7 64 bit missing recent transactions. ":
(https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-1526032160)
> see https://github.com/bitcoin/bitcoin/pull/25880

This is a wallet issue, so the ref seems wrong?

💬 LarryRuane commented on pull request "test: allow BITCOIN_TEST_PATH to specify working dir":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1526037535)
Yes, still hoping to get this merged. Force-pushed 5df0ed5ab789e005cb9aa0fc3ab61b7ce0b80ec8 to rebase onto the latest master (since it's been a long time)

💬 LarryRuane commented on pull request "test: allow BITCOIN_TEST_PATH to specify working dir":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1526052311)
Another force push to improve the doc slightly and to address review comments, thanks @kouloumos!

Ready for review, also ping @theStack.

💬 fjahr commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1526080724)
ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3

This is ok to merge but when I wrote that comment I meant to remove the `modinv()` function completely, e.g. https://github.com/fjahr/bitcoin/commit/75b8ba524d3dc957910bc8c0a4d1dd2b8ceaa426. You can pull this commit in here or we merge this now and I will open it as a follow-up. I think the comment in `modinv()` isn't accurate anymore after this change because python seems to use Exponentiation by Squaring and we don't really need a one-line functi
...

👋 satsie's pull request is ready for review: "rpc: add 'getnetmsgstats', new rpc to view network message statistics"
(https://github.com/bitcoin/bitcoin/pull/27534)

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179500435)
Would it be better to pass a reference of NetStats to every CNode upon initialization?

Example: https://github.com/bitcoin/bitcoin/commit/cd0c8eeb0940790b6ba83786d1c9e362d4dc4829

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179509345)
This was moved over from net.cpp. Should it be in the NetMsgType namespace? If so, what about the allNetMessageTypes array? There are places in this PR where I have to add +1 to account for the missing 'other' message type, and I'm not sure if it's appropriate for this to be a special case.

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179508199)
Now that an effort has been made to remove the friendship between CConnman and CNode (see this PR: https://github.com/bitcoin/bitcoin/pull/27257) I'm unsure if this (recording network stats on a CConnman object) belongs here.

🤔 jonatack reviewed a pull request: "rpc: add 'getnetmsgstats', new rpc to view network message statistics"
(https://github.com/bitcoin/bitcoin/pull/27534#pullrequestreview-1404629427)
A few initial thoughts, mainly about code organization.

💬 jonatack commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179538880)
Not sure if these to/from util helper methods are needed or if this is the right place for them rather than in their respective domains where available, like `node/connection_type` and `node/network` (the latter proposed in #27385).

Unrelated, can also make all new methods that return a value `[[nodiscard]]`.

💬 jonatack commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179532400)
Would it be a good idea for this class to be in its own file/compilation unit, rather than added into `net.h` (which is included in 40+ other files)?

💬 jonatack commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1179540855)
Perhaps consider putting all this new code in its own `rpc/netstats` files.