π¬ laisial commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461809037)
>personal attack with zero technical merit
>zero violation of any policy
>continuing witch hunt of some vs Luke.
>minimum courtesy
>change backed solely by dogpiled personal attacks is just crazy.
>The pitchfork carrying mob seems to be out in force
>this nothingburger PR
>the dogpile will continue
>no non-political reasoning
>lock this thread
@wizkid057 you're spamming off-topic with zero counterarguments on why the seed policy has not been violated. you're only policing the tone an
...
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461809037)
>personal attack with zero technical merit
>zero violation of any policy
>continuing witch hunt of some vs Luke.
>minimum courtesy
>change backed solely by dogpiled personal attacks is just crazy.
>The pitchfork carrying mob seems to be out in force
>this nothingburger PR
>the dogpile will continue
>no non-political reasoning
>lock this thread
@wizkid057 you're spamming off-topic with zero counterarguments on why the seed policy has not been violated. you're only policing the tone an
...
π¬ glozow commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461822880)
Calm down.
Historically, when a DNS seed was not operating properly, an issue was opened and we waited for the operator to respond.
Potential actions we can take: (0) address the problems with the DNS seed, (1) decide there is no problem (though I think it's clear that there are technical issues here and this is not just a "witch hunt"), or (2) remove the DNS seed. I agree that opening a PR to remove the seed straight out of the gate was not the best way to bring up the issue.
I've open
...
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461822880)
Calm down.
Historically, when a DNS seed was not operating properly, an issue was opened and we waited for the operator to respond.
Potential actions we can take: (0) address the problems with the DNS seed, (1) decide there is no problem (though I think it's clear that there are technical issues here and this is not just a "witch hunt"), or (2) remove the DNS seed. I agree that opening a PR to remove the seed straight out of the gate was not the best way to bring up the issue.
I've open
...
π¬ Eunovo commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2473420055)
> I've restored the `gen_test_data` / `load_test_data` logic so it's a proper regression test again. I've generated the new `rpc_getblockstats.json` file and added it to the PR.
This brings back the untested path problem.
> This is a no-op. You assign the value and the expected value from the same function, and then assert that it is equal. This changes what the test is checking.
This is similar to what I was pointing out in https://github.com/bitcoin/bitcoin/pull/33184#discussion_r239
...
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2473420055)
> I've restored the `gen_test_data` / `load_test_data` logic so it's a proper regression test again. I've generated the new `rpc_getblockstats.json` file and added it to the PR.
This brings back the untested path problem.
> This is a no-op. You assign the value and the expected value from the same function, and then assert that it is equal. This changes what the test is checking.
This is similar to what I was pointing out in https://github.com/bitcoin/bitcoin/pull/33184#discussion_r239
...
π¬ glozow commented on pull request "chainparams: remove *petertodd.net":
(https://github.com/bitcoin/bitcoin/pull/33730#issuecomment-3461843402)
Historically, when a DNS seed was not operating properly, an issue was opened and we waited for the operator to respond. Potential actions we can take: (0) address the problems with the DNS seed, (1) decide there is no problem, or (2) remove the DNS seed. I don't think it's appropriate to just jump to removal straight away.
I suggest you open an issue (see #29911 or #33734 for examples) and wait for a response from the operator.
(https://github.com/bitcoin/bitcoin/pull/33730#issuecomment-3461843402)
Historically, when a DNS seed was not operating properly, an issue was opened and we waited for the operator to respond. Potential actions we can take: (0) address the problems with the DNS seed, (1) decide there is no problem, or (2) remove the DNS seed. I don't think it's appropriate to just jump to removal straight away.
I suggest you open an issue (see #29911 or #33734 for examples) and wait for a response from the operator.
π€ furszy reviewed a pull request: "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response"
(https://github.com/bitcoin/bitcoin/pull/32517#pullrequestreview-3393853741)
ACK ab06c3f16063701cfa25504a0d3cd4d5a5eb3a97
(https://github.com/bitcoin/bitcoin/pull/32517#pullrequestreview-3393853741)
ACK ab06c3f16063701cfa25504a0d3cd4d5a5eb3a97
π¬ furszy commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2473437500)
nit:
Missing ref here `const CTxOut& txout`
```suggestion
/*is_change_func=*/[&pwallet](const CTxOut& txout) NO_THREAD_SAFETY_ANALYSIS {
```
Also, why not `EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)` instead of `NO_THREAD_SAFETY_ANALYSIS`?
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2473437500)
nit:
Missing ref here `const CTxOut& txout`
```suggestion
/*is_change_func=*/[&pwallet](const CTxOut& txout) NO_THREAD_SAFETY_ANALYSIS {
```
Also, why not `EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)` instead of `NO_THREAD_SAFETY_ANALYSIS`?
π¬ pinheadmz 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-3461865877)
> [!CAUTION]
> Personal attacks are not allowed and will result in a ban
- Be extremely clear and concise when posting on this thread.
- Every comment must relate directly to the topic described in the issue description
- Be mature, respectful and sensible.
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3461865877)
> [!CAUTION]
> Personal attacks are not allowed and will result in a ban
- Be extremely clear and concise when posting on this thread.
- Every comment must relate directly to the topic described in the issue description
- Be mature, respectful and sensible.
π¬ Rob1Ham 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-3461875545)
Calling out @kanzure's comment in [#33723 ](https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461716939) to ask the question on if there are advantages to having DNS seeds that are more conservative with their versioning.
> In the event that this DNS seed server is removed from the configuration, it's worth considering (or considering the effects of not) replacing it with another DNS seed node that [implements a conservative strategy with regards to node versions](https://delvingbitco
...
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3461875545)
Calling out @kanzure's comment in [#33723 ](https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461716939) to ask the question on if there are advantages to having DNS seeds that are more conservative with their versioning.
> In the event that this DNS seed server is removed from the configuration, it's worth considering (or considering the effects of not) replacing it with another DNS seed node that [implements a conservative strategy with regards to node versions](https://delvingbitco
...
π maflcko's pull request is ready for review: "ci: Call docker exec from Python script to fix word splitting"
(https://github.com/bitcoin/bitcoin/pull/33732)
(https://github.com/bitcoin/bitcoin/pull/33732)
π¬ Crypt-iQ commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461908503)
> The only downside of it would be not attracting macOS people to fuzzing development (?)
I was also thinking about this, and it could turn away some people from writing fuzz tests. Maybe if the docs mentioned a specific working version as suggested by @maflcko above, but then somebody would need to test these versions. I've had issues where even updating to a new macOS version isn't possible without some hacks.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461908503)
> The only downside of it would be not attracting macOS people to fuzzing development (?)
I was also thinking about this, and it could turn away some people from writing fuzz tests. Maybe if the docs mentioned a specific working version as suggested by @maflcko above, but then somebody would need to test these versions. I've had issues where even updating to a new macOS version isn't possible without some hacks.
π¬ fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3461989252)
`-md` runtime was 43 minutes with no caches (libccxx/depends/cacche).
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3461989252)
`-md` runtime was 43 minutes with no caches (libccxx/depends/cacche).
π¬ furszy commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461995390)
Itβs definitely painful to run it on Mac. I'm not against officially deprecating it if that is the general consensus but itβs quite useful to be able to troubleshoot individual crashes/timeouts locally. It would be handy to have pinned versions with specific docs on how to make it work.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461995390)
Itβs definitely painful to run it on Mac. I'm not against officially deprecating it if that is the general consensus but itβs quite useful to be able to troubleshoot individual crashes/timeouts locally. It would be handy to have pinned versions with specific docs on how to make it work.
π¬ maflcko commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3462061936)
What is the configure summary, before you start to compile? (The output of `cmake -B build -DENABLE_IPC=OFF -DBUILD_GUI=OFF -DWITH_QRENCODE=OFF`)
What is your XCode version?
You are certainly not using the brew installed boost lib.
What other boost libs are on your system?
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3462061936)
What is the configure summary, before you start to compile? (The output of `cmake -B build -DENABLE_IPC=OFF -DBUILD_GUI=OFF -DWITH_QRENCODE=OFF`)
What is your XCode version?
You are certainly not using the brew installed boost lib.
What other boost libs are on your system?
π¬ maflcko commented on pull request "random: scope environ extern to macOS, BSDs and Illumos":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2472906261)
nit: I think the comment can be kept as-is, or just refer to "Necessary on the above platforms". Otherwise, it is just one more line to maintain.
Also, I wonder if this can just be checked at configure-time, but no strong opinion.
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2472906261)
nit: I think the comment can be kept as-is, or just refer to "Necessary on the above platforms". Otherwise, it is just one more line to maintain.
Also, I wonder if this can just be checked at configure-time, but no strong opinion.
π¬ kanzure 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-3462103592)
I also have a question as to whether user agent version filtering was previously discussed as a matter of DNS seed policy. If it was, then perhaps someone would be kind enough to give a link to that discussion. This however seems to me like a smaller issue compared to the other considerations that are necessary for resolving this ticket.
> We also learned of an investigation following the hack and are unsure whether Luke has shared access to the server with investigators. It would be helpful if
...
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3462103592)
I also have a question as to whether user agent version filtering was previously discussed as a matter of DNS seed policy. If it was, then perhaps someone would be kind enough to give a link to that discussion. This however seems to me like a smaller issue compared to the other considerations that are necessary for resolving this ticket.
> We also learned of an investigation following the hack and are unsure whether Luke has shared access to the server with investigators. It would be helpful if
...
π¬ stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473721206)
I thought if I want to treat it as boolean, it might be better to introduce it as a flag with the dash prefix. Would it make more sense to you if I drop the dash and treat it as a chain name instead (where only regtest and mainnet are supported, and if absent, defaults to mainnet)?
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473721206)
I thought if I want to treat it as boolean, it might be better to introduce it as a flag with the dash prefix. Would it make more sense to you if I drop the dash and treat it as a chain name instead (where only regtest and mainnet are supported, and if absent, defaults to mainnet)?
π¬ fanquake commented on pull request "random: scope environ extern to macOS, BSDs and Illumos":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2473728115)
Re-modified the comment. Not sure there is too much value in moving this to configure.
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2473728115)
Re-modified the comment. Not sure there is too much value in moving this to configure.
π¬ stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473728951)
Yes I think it is better to have validation on this.
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473728951)
Yes I think it is better to have validation on this.
π€ Crypt-iQ reviewed a pull request: "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer"
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-3393285716)
I agree with @dergoegge that it's easy to become a HB peer if you're already the first to send unsolicited compact blocks to try and fingerprint a node's mempool. I think the benefit with this change is being more strict with what we'll accept and following the protocol closer (and I think there's even more improvements we can make). Left some test-related nits.
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-3393285716)
I agree with @dergoegge that it's easy to become a HB peer if you're already the first to send unsolicited compact blocks to try and fingerprint a node's mempool. I think the benefit with this change is being more strict with what we'll accept and following the protocol closer (and I think there's even more improvements we can make). Left some test-related nits.
π¬ Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2473220795)
nit: can remove this line, `stalling_peer` is already HB
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2473220795)
nit: can remove this line, `stalling_peer` is already HB