💬 pinheadmz commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#discussion_r1173902633)
Yeah I think thats just to decode numeric addresses like "100.200.30.40" etc. It looked to me like if an actual DNS request is necessary it has its own methods to make reqests directly to the nameservers provided by the platform? When things get "serious":
https://github.com/libevent/libevent/blob/master/evdns.c#L5623
(https://github.com/bitcoin/bitcoin/pull/27505#discussion_r1173902633)
Yeah I think thats just to decode numeric addresses like "100.200.30.40" etc. It looked to me like if an actual DNS request is necessary it has its own methods to make reqests directly to the nameservers provided by the platform? When things get "serious":
https://github.com/libevent/libevent/blob/master/evdns.c#L5623
💬 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.
(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.
(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.
(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
(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
...
(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
(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?
(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
(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
(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.
(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
...
(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
...
(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
...
(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
...
(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'
...
(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.
(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,
...
(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
...
(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?
(https://github.com/bitcoin/bitcoin/issues/27512#issuecomment-1518085815)
Dupe of #16733?