💬 mzumsande commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1518004494)
Concept ACK. Not sure if this should be the default or an optional mode (in some cases, a higher chance of transmission may be more important than privacy).

It might also be helpful to combine this with some heuristics that the tx percolated through the network:
For example, require that 2 of your regular full outbound connections (that you didn't send it to ) have inv'ed the tx back to you - otherwise try again after some time.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173921394)
The blockman and chainstate instances may be deleted during the lifetime of the zmq instance similarly as described in mzsumsande's [comment](https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1160180267) when iterating the for loop. If we pass a reference to the `NodeContext` instead, we have better guarantees that its blockman reference points to an actual object.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173922264)
Good catch, thank you! Will also remove the `<args.h>` include.

🤔 brunoerg reviewed a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1396013005)
Concept ACK

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518024541)
Thank you for the review @dergoegge,

Updated 1a7d73d30e312e862be4560601302e79725b1bc7 -> ed3159e0eb105d9f46659bd8f8b2db27d94841de ([removeBlockstorageArgs_14](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_14) -> [removeBlockstorageArgs_15](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_15), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_14..removeBlockstorageArgs_15))
* Addressed @dergoegge's [comment](https://gith
...

💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1173930525)
Done on the latest push

💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173938033)
Is that actually possible? Seems like all the validation interfaces including the zmq one are unregistered before chainman is destroyed?

So it can't be holding a reference to blockman that is actually destroyed, see:

https://github.com/bitcoin/bitcoin/blob/f3f5c9712670fc2c9fb3b57580146aa248a5d36f/src/init.cpp#L327-L343

Or are you just saying that having a reference to the node context has better lifetime guarantees because basically nothing outlives the node context?

💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518037638)
Code review ACK bab12a6a029bed86ddcc15d42a236a855e277e74

💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1518046601)
Code review ACK ed3159e0eb105d9f46659bd8f8b2db27d94841de

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1173954405)
> Is that actually possible?

Likely that I am missing something in the logic. The way I see it each iteration of the for loop creates and destroys the `chainman` by calling `std::make_unique` again, while the `Shutdown` code is not called during each iteration.

👍 TheCharlatan approved a pull request: "build: use latest config.{guess,sub} in depends"
(https://github.com/bitcoin/bitcoin/pull/27508#pullrequestreview-1396060103)
ACK 4a3f1db4ea5f90277cf7f57c051a2285e8b42468

I also checked the content from
```
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' -O config.guess
wget 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.sub;hb=HEAD' -O config.sub
```

Guix builds:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
fef9152d593aa1cb3243f37b8c069ef863f4e270962e17918cfa8
...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173964808)
> Seems kind of weird to change the CLI to use a "test-only" RPC in any case

@fanquake, @amitiuttarwar `-generate` CLI currently [uses](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/bitcoin-cli.cpp#L1178) a [hidden RPC](https://github.com/bitcoin/bitcoin/blob/467fa8943801911c233cb96d45282b1de10bfa90/src/rpc/mining.cpp#L1055) (`generatetoaddress`) too.
i don't think a normal user would be interested in the new/tried table breakdown of addresses. so hop
...

📝 stratospher opened a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511)
implements https://github.com/bitcoin/bitcoin/issues/26907. split off from #26988 to keep RPC, CLI discussions separate.

This PR introduces a new RPC `getaddrmaninfo`which returns the count of addresses in the new/tried table of a node's addrman broken down by network type. This would be useful for users who want to see the distribution of addresses from different networks across new/tried table in the addrman.

```jsx
$ getaddrmaninfo

Result:
{ (json object) json obj
...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173965885)
> Maybe borrow from the current -addrinfo help, along the lines of CLI -addrinfo previously returned addresses known to the node after filtering for quality and recency. It now returns all of the addresses known to the node.

thanks @jonatack! this sounds much better.

> It might be interesting to have both with and without IsTerrible() filtering to maintain the current info returned and to be able to compare over time, either by returning a breakdown in the new RPC or by leaving -addrinfo u
...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173966799)
Getting stats on filtered/unfiltered addresses is interesting data!

But also feeling confused about what to do here - so haven't changed anything yet. (updated the PR to just discuss CLI changes, opened https://github.com/bitcoin/bitcoin/pull/27511 for getaddrmaninfo RPC)

1. what happens in the long run? In the long run, wouldn't ordinary users find it better if the CLI has a simple display of just breakdown of addresses by network? they wouldn't care much about filtering. I'd imagine we'
...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173967154)
you're right! I've updated `getaddrmaninfo` taking both these suggestions.

⚠️ dongcarl opened an issue: "user config: Support XDG Base Directory Specification or `$HOME/.{config,local}/`"
(https://github.com/bitcoin/bitcoin/issues/27512)
The [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html) has been around for a while and there seems to be growing support for it in various applications. This issue is to start discussion around **if** and **how** we should support it.

If the project is interested in supporting it (or a subset of it), here's an example of what we could do for the config file:

1. Check for config files in the following order (first hit is applied,
...

💬 stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1518075524)
thank you for the useful feedback! Splitting this into 2 separate PRs since it’d be easier to think about RPC and CLI parts separately. I’ve opened https://github.com/bitcoin/bitcoin/pull/27511 for getaddrmaninfo RPC and updated this PR to reflect just the CLI changes.

- I liked returning objects as output and displaying total addresses per network ideas. Updated getaddrmaninfo in https://github.com/bitcoin/bitcoin/pull/27511 to use these.
- Haven't changed the previous CLI approach because
...

💬 fanquake commented on issue "user config: Support XDG Base Directory Specification or `$HOME/.{config,local}/`":
(https://github.com/bitcoin/bitcoin/issues/27512#issuecomment-1518085815)
Dupe of #16733?

💬 jonatack commented on pull request "cli: rework -addrinfo cli to use addresses which aren’t filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173977053)
Please don't break user space. Again suggest https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1147473618.

💬 jonatack commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1518103852)
This PR is based on master from two months age. You may need to rebase it to current master for the CI to be green.