💬 fanquake commented on issue "Erlay status in getpeerinfo":
(https://github.com/bitcoin/bitcoin/issues/26602#issuecomment-1571656524)
Closing for now. The comment here: https://github.com/bitcoin/bitcoin/pull/27797#issuecomment-1570503893, is a good summary of why we aren't going to do this. You can run with the patch from that PR, or look at the p2p log.
(https://github.com/bitcoin/bitcoin/issues/26602#issuecomment-1571656524)
Closing for now. The comment here: https://github.com/bitcoin/bitcoin/pull/27797#issuecomment-1570503893, is a good summary of why we aren't going to do this. You can run with the patch from that PR, or look at the p2p log.
✅ fanquake closed an issue: "Erlay status in getpeerinfo"
(https://github.com/bitcoin/bitcoin/issues/26602)
(https://github.com/bitcoin/bitcoin/issues/26602)
💬 fanquake commented on issue "fuzz: mini_miner: Timeout in mini_miner":
(https://github.com/bitcoin/bitcoin/issues/27799#issuecomment-1571665679)
cc @Xekyo
(https://github.com/bitcoin/bitcoin/issues/27799#issuecomment-1571665679)
cc @Xekyo
💬 fanquake commented on pull request "rpc, net: add erlay status in `getpeerinfo`":
(https://github.com/bitcoin/bitcoin/pull/27797#issuecomment-1571669565)
I think you could leave a comment in #21515, about cherry-picking this change into that PR, but I don't think there's a need to leave this PR open, if we aren't going to merge it. I agree with Martins comment above; it's premature to add this, and probably not something we should add to our RPC interface just as a convenience for developers, for testing a not-yet-implemented/experimental feature.
(https://github.com/bitcoin/bitcoin/pull/27797#issuecomment-1571669565)
I think you could leave a comment in #21515, about cherry-picking this change into that PR, but I don't think there's a need to leave this PR open, if we aren't going to merge it. I agree with Martins comment above; it's premature to add this, and probably not something we should add to our RPC interface just as a convenience for developers, for testing a not-yet-implemented/experimental feature.
✅ fanquake closed a pull request: "rpc, net: add erlay status in `getpeerinfo`"
(https://github.com/bitcoin/bitcoin/pull/27797)
(https://github.com/bitcoin/bitcoin/pull/27797)
💬 MarnixCroes commented on pull request "doc: Tor: fix link & generalize onion getnodeaddresses RPC":
(https://github.com/bitcoin/bitcoin/pull/27719#issuecomment-1571677493)
@jonatack thank you! I've applied your suggestions
(https://github.com/bitcoin/bitcoin/pull/27719#issuecomment-1571677493)
@jonatack thank you! I've applied your suggestions
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1571690382)
`>>` This is ready for review and testing `<<`
> ... if the transaction is sent along with the potentially uniquely identifying UA comment, or other fingerprints in the version message. Maybe it makes sense to define a static version message ...
@MarcoFalke, I went with a [completely static `VERSION` message](https://github.com/vasild/bitcoin/blob/b73810f80917dd882d6161f6892f10f8c46a8193/src/net_processing.cpp#L1439-L1446)
> ... Perhaps it's worth mimicking `bitcoin-submittx`
@sipa,
...
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1571690382)
`>>` This is ready for review and testing `<<`
> ... if the transaction is sent along with the potentially uniquely identifying UA comment, or other fingerprints in the version message. Maybe it makes sense to define a static version message ...
@MarcoFalke, I went with a [completely static `VERSION` message](https://github.com/vasild/bitcoin/blob/b73810f80917dd882d6161f6892f10f8c46a8193/src/net_processing.cpp#L1439-L1446)
> ... Perhaps it's worth mimicking `bitcoin-submittx`
@sipa,
...
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1212891740)
This, together with the other change from commit 0ec7b979e7a9d4c30a97c39c2a64768d7c5662b1 `net_processing: omit unbroadcast txs from replies to GETDATA and MEMPOOL` alter the behavior even if sensitive relay is not used (e.g. disabled or Tor and I2P not reachable).
I think it is beneficial in that case too, but is not the purpose of this PR to improve that. Should this be guarded by `if (UseSensitiveRelay())`?
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1212891740)
This, together with the other change from commit 0ec7b979e7a9d4c30a97c39c2a64768d7c5662b1 `net_processing: omit unbroadcast txs from replies to GETDATA and MEMPOOL` alter the behavior even if sensitive relay is not used (e.g. disabled or Tor and I2P not reachable).
I think it is beneficial in that case too, but is not the purpose of this PR to improve that. Should this be guarded by `if (UseSensitiveRelay())`?
👋 vasild's pull request is ready for review: "Relay own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/27509)
(https://github.com/bitcoin/bitcoin/pull/27509)
💬 fanquake commented on pull request "doc: Tor: fix link & generalize onion getnodeaddresses RPC":
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1212894727)
`These results, as well as those of -addrinfo, are filtered for quality and recency.` - can you drop this. It's not clear why we need to mention an implemention detail of `-addrinfo` here.
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1212894727)
`These results, as well as those of -addrinfo, are filtered for quality and recency.` - can you drop this. It's not clear why we need to mention an implemention detail of `-addrinfo` here.
💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212736489)
ee6e289:
```suggestion
def __rsub__(self, a):
```
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212736489)
ee6e289:
```suggestion
def __rsub__(self, a):
```
💬 stratospher commented on pull request "Introduce secp256k1 module with field and group classes to test framework":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212950016)
> The way FE values are represented inside the class is an unobservable implementation detail, so calling int(FE) doesn't "update" the FE object in any observable way - it just makes future calls more efficient.
thinking of it this way makes sense. thanks!
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1212950016)
> The way FE values are represented inside the class is an unobservable implementation detail, so calling int(FE) doesn't "update" the FE object in any observable way - it just makes future calls more efficient.
thinking of it this way makes sense. thanks!
💬 brandonpille commented on issue "rpc: Allow importing wallets by data instead of by filename":
(https://github.com/bitcoin/bitcoin/issues/27597#issuecomment-1571814230)
No it's ok
(https://github.com/bitcoin/bitcoin/issues/27597#issuecomment-1571814230)
No it's ok
✅ fanquake closed an issue: "rpc: Allow importing wallets by data instead of by filename"
(https://github.com/bitcoin/bitcoin/issues/27597)
(https://github.com/bitcoin/bitcoin/issues/27597)
💬 MarnixCroes commented on pull request "doc: Tor: fix link & generalize onion getnodeaddresses RPC":
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1212994429)
done
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1212994429)
done
💬 theuni commented on pull request "depends: modernize clang flags for Darwin":
(https://github.com/bitcoin/bitcoin/pull/27798#discussion_r1213032495)
ffs...
(https://github.com/bitcoin/bitcoin/pull/27798#discussion_r1213032495)
ffs...
💬 jonatack commented on pull request "doc: Tor: fix link & generalize onion getnodeaddresses RPC":
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1213064407)
> It's not clear why we need to mention an implemention detail of `-addrinfo` here.
That is how RPC `getnodeaddresses` works as well -- see its help.
Would suggest either reverting to the previous push, or replacing this paragraph in tor/i2p/cjdns.md with something like the following:
You can use the `getnodeaddresses` RPC to fetch a number of onion peers known to your node; run `bitcoin-cli help getnodeaddresses` for details.
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1213064407)
> It's not clear why we need to mention an implemention detail of `-addrinfo` here.
That is how RPC `getnodeaddresses` works as well -- see its help.
Would suggest either reverting to the previous push, or replacing this paragraph in tor/i2p/cjdns.md with something like the following:
You can use the `getnodeaddresses` RPC to fetch a number of onion peers known to your node; run `bitcoin-cli help getnodeaddresses` for details.
💬 jonatack commented on pull request "doc: Tor: fix link & generalize onion getnodeaddresses RPC":
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1213069877)
Now that the link has been removed, please update the PR title and description from "fix" to "remove".
(https://github.com/bitcoin/bitcoin/pull/27719#discussion_r1213069877)
Now that the link has been removed, please update the PR title and description from "fix" to "remove".
💬 glozow commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1571967192)
> Because a -fallback fee already exists. Why can't it exist for other use cases?
Node and wallet are 2 separate components; it doesn't makes sense for a wallet option to bleed into node behavior. Another example: the wallet setting `-walletrejectlongchains=0` does not mean the mempool should also accept long chains of transactions, and the existence of that wallet option does not mean we should also add that option to the node. If inheritance exists, the wallet's options should be informed b
...
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1571967192)
> Because a -fallback fee already exists. Why can't it exist for other use cases?
Node and wallet are 2 separate components; it doesn't makes sense for a wallet option to bleed into node behavior. Another example: the wallet setting `-walletrejectlongchains=0` does not mean the mempool should also accept long chains of transactions, and the existence of that wallet option does not mean we should also add that option to the node. If inheritance exists, the wallet's options should be informed b
...
💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1571998659)
Side note: I tried centos and it also failed with the same error as debian. Commit:
<details><summary>diff</summary>
```diff
commit 57903b154ec1dc5d86f174d311e9875bbf0c4106
Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu Jun 1 13:48:41 2023 +0200
ci: Use centos to work around fuzz msan debian bug
diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh
index dd694f818c..5aca7a9be9 100755
--- a/ci/test/00_setu
...
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1571998659)
Side note: I tried centos and it also failed with the same error as debian. Commit:
<details><summary>diff</summary>
```diff
commit 57903b154ec1dc5d86f174d311e9875bbf0c4106
Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Thu Jun 1 13:48:41 2023 +0200
ci: Use centos to work around fuzz msan debian bug
diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh
index dd694f818c..5aca7a9be9 100755
--- a/ci/test/00_setu
...