💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1837989979)
Yeah, adding `mempool.m_opts.require_standard` to the if condition would make sense - `acceptnonstdtxn` should turn off all dust checks including this one imo

🤔 ryanofsky reviewed a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2429375066)
Updated ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba -> fe39acf88ff552bfc4a502c99774375b91824bb1 ([`pr/tcheck.4`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.4) -> [`pr/tcheck.5`](https://github.com/ryanofsky/bitcoin/commits/pr/tcheck.5), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/tcheck.4..pr/tcheck.5)) with suggested changes. Thanks for the reviews and suggestions!

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837966979)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950

> As you say, only the length and type are no longer checked. This PR now implements rudimentary checking of flags, width and precision.

Thanks, applied suggestion

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837996485)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835766554

> we might as well extract number parsing to a local lambda like you did with the other ones.

This seems reasonable but I"m not sure it's clearer, and it does make the diff bigger replacing the `maybe_num` code that doesn't have to change currently. Happy to apply this suggestion if other reviews think it is a good idea.

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837973744)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834499010

Thanks, added

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837974075)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834504076

Thanks, added

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837983669)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770704

I can apply the `"#0- +"sv.find` change if others like it, but to me it seems less readable and only a little shorter.

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1837980148)
re: https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1835770569

Nice suggestion! Applied

💬 ryanofsky commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#issuecomment-2470441887)
re: https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2425834191

> Also, not exactly sure why `%n` parity wasn't added

Thanks I hadn't seen #30999, and it seems like that would be a reasonable thing to check for, though I think there is a case for keeping the code as simple as possible and not trying to reproduce tinyformat quirks here. But the reason for not making that change here is I don't think it's related to this PR, and I think it's generally better to make separate c
...

🤔 glozow reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2429428705)
ACK 131bed19bdfc922328cad9781fa9122b6810a8fd

Please lmk what you want to do with the remaining comments, I'm happy to reack or merge with a followup lined up.

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838062023)
Did you consider benching with the parent in package? I suppose it's about the same.

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1837998371)
They aren't no-ops. You should be checking `getprioritisedtransactions` instead. You can prio things that aren't in mempool yet, and mempool tracks them so it can be used in validation later.

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1838074248)
Ah RIP, this is a merge conflict with #30592. That's the CI error.

💬 ryanofsky commented on pull request "util: Narrow scope of args (-color, -version, -conf, -datadir)":
(https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2470510859)
Code review d9cfb2a314caa1cdcae8e47a03dad557ad96e069.

This generally looks good except commit 2d65e6ae0756b87e82dbafd032308e8687ec7941 disallowing `-noconf`. Specifying `-noconf` to disable reading from the default config file path is useful and something that should be supported, even if now it's only working in a very quirky way by treating the datadir as an empty configuration file. I think a better implementation would not look too different though, maybe like:

```diff
--- a/src/commo
...

💬 AndreaDiazCorreia commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-2470534984)
I've reviewed and tested the listmempooltransactions RPC implementation. Here's what I did:

- Pulled the branch and rebased it on top of master
- Tested it manually on regtest both with and without transactions in the mempool
- Created a functional test for this RPC. You can find it in my repo at https://github.com/AndreaDiazCorreia/bitcoin/tree/PR29016-listmempooltransactions

The test verifies:
- Basic mempool sequence behavior
- Transaction tracking
- Sequence filtering functionalit
...

💬 vasild commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2470557837)
Right! Looks like for `Discover()` to run `-discover` must be enabled and `-bind` not used. It is tough to explain what a config like `-discover=1 -bind=1.2.3.4:5555` would do. Seems that this is another thing that can be simplified.

Also, this has another flaw: `bind_on_any` will be `false` if `-bind=0.0.0.0:5555` is used, but it should really be `true` :-/

💬 vasild commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2470593900)
@darosior, good observations and questions! Maybe use the P2P-seeds only as a fallback if it takes too long?

I mean this - right now there are around 2000 hardcoded addresses in `contrib/seeds/nodes_main.txt`. Some of them will be down, thus the problem mentioned above: "Another downside would be that finding an initial batch of peers would take longer". So, maybe if a new node can't find enough peers in some reasonable time from those 2000 (and GETADDR replies if it manages to connect to som
...

💬 vasild commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1838156911)
I meant `-bind=` as a means to disable the onion bind on `127.0.0.1:8334` to avoid the port collision, but I typed `-bind=...=onion` instead :(

One can use both depending on what's needed: `-bind=1.2.3.4:5555 -bind=127.0.0.1:8335=onion`.

📝 theStack opened a pull request: "doc: mention `descriptorprocesspsbt` in psbt.md"
(https://github.com/bitcoin/bitcoin/pull/31277)
Noticed that the `descriptorprocesspsbt` RPC call is currently not documented anywhere.

💬 vasild commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1838170305)
> Do you think that is an issue we should care about?

No.

I have found simple/dumb programs easier to configure compared to programs that try to be intelligent on behalf of the user.

> Shaw's Principle:
> Build a system that even a fool can use, and only a fool will want to use it.

This is a bit subjective and I may have a bias here, but if I put `Port 80` in `/etc/ssh/sshd_config` I can't imagine sshd saying something like "Hey, you are binding sshd on port 80, your web ser
...