💬 00w1 commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3462899397)
> Luke-jr's seed returns knots 29.2 nodes, and yet he claims knots 29.2 is based on the same code it currently won't return. https://x.com/LukeDashjr/status/1977355254510256135 even though the issue is now known to luke-jr the behavior hasn't been corrected.
I have been trying to resolve `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` since last 15 minutes but it did not return any knots v29.2 node IP address in the response. Others who reviewed https://github.com/bitcoin/bitcoin/pull/33723 did n
...
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3462899397)
> Luke-jr's seed returns knots 29.2 nodes, and yet he claims knots 29.2 is based on the same code it currently won't return. https://x.com/LukeDashjr/status/1977355254510256135 even though the issue is now known to luke-jr the behavior hasn't been corrected.
I have been trying to resolve `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` since last 15 minutes but it did not return any knots v29.2 node IP address in the response. Others who reviewed https://github.com/bitcoin/bitcoin/pull/33723 did n
...
💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3462919053)
@hebasto tried that but fails.
configure.log https://gist.github.com/umrashrf/691047f40e17b7ca06abce2d25375323
build.log https://gist.github.com/umrashrf/019e10e3fca1c6a686b556c7a6b65ce2
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3462919053)
@hebasto tried that but fails.
configure.log https://gist.github.com/umrashrf/691047f40e17b7ca06abce2d25375323
build.log https://gist.github.com/umrashrf/019e10e3fca1c6a686b556c7a6b65ce2
⚠️ 00w1 opened an issue: "Remove *petertodd.net DNS seed for testnet and mainnet"
(https://github.com/bitcoin/bitcoin/issues/33736)
Creating an issue based on @glozow's suggestion in https://github.com/bitcoin/bitcoin/pull/33730#issuecomment-3461843402
Some [emails](https://agorism.dev/petertodd-emails.txt) were leaked on bitcointalk and Peter Todd acknowledged that they are not fake. His communication with someone about bitcoin development and privacy who claims to work for government agencies potentially violates the DNS seed policy.
> A DNS seed operating organization or person is expected to follow good host security p
...
(https://github.com/bitcoin/bitcoin/issues/33736)
Creating an issue based on @glozow's suggestion in https://github.com/bitcoin/bitcoin/pull/33730#issuecomment-3461843402
Some [emails](https://agorism.dev/petertodd-emails.txt) were leaked on bitcointalk and Peter Todd acknowledged that they are not fake. His communication with someone about bitcoin development and privacy who claims to work for government agencies potentially violates the DNS seed policy.
> A DNS seed operating organization or person is expected to follow good host security p
...
💬 achow101 commented on issue "Remove *petertodd.net DNS seed for testnet and mainnet":
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3463006389)
Can you cite where in those emails you believe this part of the policy is being violated? I searched for "dns" and "seed" and got no results. I searched for "server" and don't see anything that seems to discuss handing over any DNS seed infrastructure to someone else.
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3463006389)
Can you cite where in those emails you believe this part of the policy is being violated? I searched for "dns" and "seed" and got no results. I searched for "server" and don't see anything that seems to discuss handing over any DNS seed infrastructure to someone else.
👍 pinheadmz approved a pull request: "http: replace WorkQueue and single threads handling for ThreadPool"
(https://github.com/bitcoin/bitcoin/pull/33689#pullrequestreview-3395201777)
ACK f5eca8d5252a68f0fbc8b5efec021c75744555b6
minor changes since last ack
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK f5eca8d5252a68f0fbc8b5efec021c75744555b6
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmkCWuYACgkQ5+KYS2KJ
yTpb9RAAgxNlDQlqkVfnIK2Xt8BQmRsiG+1KHQGJA2d3RHoxjFg2129fj47y5i+t
GlatlcsXEcibI2D0F22sKSzY1u0LWy4GYLI1d12JAIewA99n/lRr1ktDk3v64pp8
kldvYpd8UBs7DCHSJhPs4HbOPgwIILPASdSbQTb+6m48X5jg+cu+yMBu
...
(https://github.com/bitcoin/bitcoin/pull/33689#pullrequestreview-3395201777)
ACK f5eca8d5252a68f0fbc8b5efec021c75744555b6
minor changes since last ack
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK f5eca8d5252a68f0fbc8b5efec021c75744555b6
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmkCWuYACgkQ5+KYS2KJ
yTpb9RAAgxNlDQlqkVfnIK2Xt8BQmRsiG+1KHQGJA2d3RHoxjFg2129fj47y5i+t
GlatlcsXEcibI2D0F22sKSzY1u0LWy4GYLI1d12JAIewA99n/lRr1ktDk3v64pp8
kldvYpd8UBs7DCHSJhPs4HbOPgwIILPASdSbQTb+6m48X5jg+cu+yMBu
...
🤔 polespinasa reviewed a pull request: "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates"
(https://github.com/bitcoin/bitcoin/pull/33199#pullrequestreview-3394935535)
Just some nits and questions :=)
(https://github.com/bitcoin/bitcoin/pull/33199#pullrequestreview-3394935535)
Just some nits and questions :=)
💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474344669)
Is a1a1bfa2445da11369323b649253f81d288f2813 necessary?
Won't old versions be able to read the file even if estimations are sub 1sat/vb?
just curiosity, how is this number chosen? Related to release number?
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474344669)
Is a1a1bfa2445da11369323b649253f81d288f2813 necessary?
Won't old versions be able to read the file even if estimations are sub 1sat/vb?
just curiosity, how is this number chosen? Related to release number?
💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474331227)
nit: I would keep the original first sentence: `The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we might want to track` + `which should be DEFAULT_MIN_RELAY_TX_FEE.`
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474331227)
nit: I would keep the original first sentence: `The MIN_BUCKET_FEERATE should just be set to the lowest reasonable feerate we might want to track` + `which should be DEFAULT_MIN_RELAY_TX_FEE.`
💬 polespinasa commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474566135)
this changes feel out of scope for this pr
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2474566135)
this changes feel out of scope for this pr
💬 gmaxwell commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3463141404)
Odd. I connected to it in a loop and eventually got a "/Satoshi:29.2.0/Knots:20251010/" -- also other knots versions and yet yours shows no knots at all?
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3463141404)
Odd. I connected to it in a loop and eventually got a "/Satoshi:29.2.0/Knots:20251010/" -- also other knots versions and yet yours shows no knots at all?
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474671206)
true. Closing this.
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474671206)
true. Closing this.
💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474735112)
I mean pick up that it's being written by multiple threads if something breaks the assumptions in this test. It seems like an ok idea. Or am I wrong?
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474735112)
I mean pick up that it's being written by multiple threads if something breaks the assumptions in this test. It seems like an ok idea. Or am I wrong?
💬 Sjors commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3463296694)
I share the concerns, but at the same time it will be a while before we ship a new release. So I prefer to just wait a few days / weeks to hear a clear answer from Luke himself, preferably here on Github and not some gated social media platform that I can't read :-)
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3463296694)
I share the concerns, but at the same time it will be a while before we ship a new release. So I prefer to just wait a few days / weeks to hear a clear answer from Luke himself, preferably here on Github and not some gated social media platform that I can't read :-)
💬 polespinasa commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-3463307304)
Is this necessary? doesn't #31664 and the mempool forecaster fix this?
If the block estimator does not have enough info, it can fallback to the mempool forecaster that will return the min feerate to enter the block template it creates?
(https://github.com/bitcoin/bitcoin/pull/32395#issuecomment-3463307304)
Is this necessary? doesn't #31664 and the mempool forecaster fix this?
If the block estimator does not have enough info, it can fallback to the mempool forecaster that will return the min feerate to enter the block template it creates?
💬 ajtowns commented on pull request "doc: better document NetEventsInterface and the deletion of "CNode"s":
(https://github.com/bitcoin/bitcoin/pull/32278#discussion_r2474399807)
I think this would be better phrased as requirements for implementers of the interface, rather than the user of the interface. ie: "For a given `CNode`, the methods will be called in the following order:". (Both `CNode` and `NetEventsInterface` are interfaces the net module offers, so the appropriate documentation is how they work and what you need to do to implement NetEventsInterface, not how net.cpp needs to be implemented)
(https://github.com/bitcoin/bitcoin/pull/32278#discussion_r2474399807)
I think this would be better phrased as requirements for implementers of the interface, rather than the user of the interface. ie: "For a given `CNode`, the methods will be called in the following order:". (Both `CNode` and `NetEventsInterface` are interfaces the net module offers, so the appropriate documentation is how they work and what you need to do to implement NetEventsInterface, not how net.cpp needs to be implemented)
💬 ajtowns commented on pull request "doc: better document NetEventsInterface and the deletion of "CNode"s":
(https://github.com/bitcoin/bitcoin/pull/32278#discussion_r2474449300)
I'd phrase this as:
Destroying objects from this list works as follows (see DisconnectNodes(), DeleteNode()):
* we will close the socket, release locks, and reset global info relating to the connection
* the node will be moved from `m_nodes` to `m_nodes_disconnected`, preventing future calls to `ProcessMessages()` and `SendMessages()` for this node, and decrementing the reference count
* we will wait for GetRefCount() to return `<= 0` indicating there are no other references to this nod
...
(https://github.com/bitcoin/bitcoin/pull/32278#discussion_r2474449300)
I'd phrase this as:
Destroying objects from this list works as follows (see DisconnectNodes(), DeleteNode()):
* we will close the socket, release locks, and reset global info relating to the connection
* the node will be moved from `m_nodes` to `m_nodes_disconnected`, preventing future calls to `ProcessMessages()` and `SendMessages()` for this node, and decrementing the reference count
* we will wait for GetRefCount() to return `<= 0` indicating there are no other references to this nod
...
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474919127)
> I mean pick up that it's being written by multiple threads if something breaks the assumptions in this test. It seems like an ok idea. Or am I wrong?
I don't think there is any broken assumption. `BlockWorkers()` ensures that all threads except one are blocked. We could go further and check that all tasks submitted post the `BlockWorkers` call are executed by the same thread too, but that seems to be an overkill to me. We ensure that in another way.
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2474919127)
> I mean pick up that it's being written by multiple threads if something breaks the assumptions in this test. It seems like an ok idea. Or am I wrong?
I don't think there is any broken assumption. `BlockWorkers()` ensures that all threads except one are blocked. We could go further and check that all tasks submitted post the `BlockWorkers` call are executed by the same thread too, but that seems to be an overkill to me. We ensure that in another way.
💬 ajtowns commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2474923790)
Why monitor cpu time rather than real time? Isn't time spent waiting for disk access while looking up the utxo set equally problematic? Waiting on another thread's lock is arguably less problematic (it's the other thread that's delaying things), but if an attacker is deliberately triggering lock contention somehow to delay processing other thread's messages, that would still be a problem.
Adding OS-specific logic rather than just using either real time or a steady clock seems over-complicated
...
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2474923790)
Why monitor cpu time rather than real time? Isn't time spent waiting for disk access while looking up the utxo set equally problematic? Waiting on another thread's lock is arguably less problematic (it's the other thread that's delaying things), but if an attacker is deliberately triggering lock contention somehow to delay processing other thread's messages, that would still be a problem.
Adding OS-specific logic rather than just using either real time or a steady clock seems over-complicated
...
💬 mzumsande commented on pull request "p2p: reduce false-positives in addr rate-limiting":
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3463416565)
> If we are being connected to, I think our self-announcement won't be rate-limited because we'll wait until receiving getaddr, call SetupAddressRelay, and then in SendMessages, our self-announcement will replace one of the existing entries in the getaddr response. There's no time-gap in this case afaict.
True, I have observed this effect too recently, it isn't really clean though - I don't think anyone intended it to work such that a GETADDR answers would contain 999 addresses from addrman a
...
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3463416565)
> If we are being connected to, I think our self-announcement won't be rate-limited because we'll wait until receiving getaddr, call SetupAddressRelay, and then in SendMessages, our self-announcement will replace one of the existing entries in the getaddr response. There's no time-gap in this case afaict.
True, I have observed this effect too recently, it isn't really clean though - I don't think anyone intended it to work such that a GETADDR answers would contain 999 addresses from addrman a
...
💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2475026718)
The explicit was suggested in https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2226976728. As far as I know it's good practice, so that the compiler can't make implicit conversions to resolve the parameters of a function. Also see: https://github.com/bitcoin/bitcoin/blob/292ea0eb8982faef460c210bd4215d603f487463/doc/developer-notes.md?plain=1#L835
The initializer list was suggested in https://github.com/bitcoin/bitcoin/pull/25968#pullrequestreview-1096400728, I think it's consistent w
...
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2475026718)
The explicit was suggested in https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2226976728. As far as I know it's good practice, so that the compiler can't make implicit conversions to resolve the parameters of a function. Also see: https://github.com/bitcoin/bitcoin/blob/292ea0eb8982faef460c210bd4215d603f487463/doc/developer-notes.md?plain=1#L835
The initializer list was suggested in https://github.com/bitcoin/bitcoin/pull/25968#pullrequestreview-1096400728, I think it's consistent w
...