💬 eval-exec commented on pull request "torcontrol: Add comment explaining Proxy credential randomization for Tor privacy":
(https://github.com/bitcoin/bitcoin/pull/31973#discussion_r1977790296)
I executed
```bash
docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
```
it say:
```
Success: no issues found in 305 source files
```
(https://github.com/bitcoin/bitcoin/pull/31973#discussion_r1977790296)
I executed
```bash
docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
```
it say:
```
Success: no issues found in 305 source files
```
🤔 1440000bytes reviewed a pull request: "rpc: add cli examples, update docs"
(https://github.com/bitcoin/bitcoin/pull/31958#pullrequestreview-2654617253)
ACK https://github.com/bitcoin/bitcoin/pull/31958/commits/0ad066c85a46208041f816d4edba36e8712763e9
Related PR: https://github.com/bitcoin/bitcoin/pull/27454
(https://github.com/bitcoin/bitcoin/pull/31958#pullrequestreview-2654617253)
ACK https://github.com/bitcoin/bitcoin/pull/31958/commits/0ad066c85a46208041f816d4edba36e8712763e9
Related PR: https://github.com/bitcoin/bitcoin/pull/27454
📝 Sjors opened a pull request: "Drop testnet3"
(https://github.com/bitcoin/bitcoin/pull/31974)
Testnet3 was deprecated in v28 with the introduction of testnet4. Dropping it in v30 seems reasonable. Note that v29 will be supported for a while.
There is no more `[test]` network section. The user is expected to set testnet4 explicitly, which should make future rotation of testnets easier.
If a `[test]` or `[testnet3]` section is present in `bitcoin.conf` a warning is issued:
```
Warning: .../bitcoin.conf:18 Section [test] is not recognized.
```
Starting with `-testnet3` will be
...
(https://github.com/bitcoin/bitcoin/pull/31974)
Testnet3 was deprecated in v28 with the introduction of testnet4. Dropping it in v30 seems reasonable. Note that v29 will be supported for a while.
There is no more `[test]` network section. The user is expected to set testnet4 explicitly, which should make future rotation of testnets easier.
If a `[test]` or `[testnet3]` section is present in `bitcoin.conf` a warning is issued:
```
Warning: .../bitcoin.conf:18 Section [test] is not recognized.
```
Starting with `-testnet3` will be
...
🤔 1440000bytes reviewed a pull request: "rpc: Allow fullrbf fee bump in (psbt)bumpfee"
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2654624537)
utACK https://github.com/bitcoin/bitcoin/pull/31953/commits/fa16051eac9a4bc1a7e0234b65d615e655ff7cf0
(https://github.com/bitcoin/bitcoin/pull/31953#pullrequestreview-2654624537)
utACK https://github.com/bitcoin/bitcoin/pull/31953/commits/fa16051eac9a4bc1a7e0234b65d615e655ff7cf0
💬 instagibbs commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694932831)
I've said it elsewhere, but I think it deserves a quite long deprecation cycle, along with some data gathering.
We don't want services not upgrading for fixes because their test environment would break.
e.g., what exchanges with testnet environments have adopted testnet4 (or signet), much less would be ok with removing testnet3?
Does btcd support it? LND? Eclaire? CLN? etc.
Lots of work to be done here imo.
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694932831)
I've said it elsewhere, but I think it deserves a quite long deprecation cycle, along with some data gathering.
We don't want services not upgrading for fixes because their test environment would break.
e.g., what exchanges with testnet environments have adopted testnet4 (or signet), much less would be ok with removing testnet3?
Does btcd support it? LND? Eclaire? CLN? etc.
Lots of work to be done here imo.
💬 1440000bytes commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2694939107)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-2694939107)
Concept ACK
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694940376)
Let's open a separate RFC to discuss when to drop testnet3. I can rebase this for a while if we decide v30 is too early.
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2694940376)
Let's open a separate RFC to discuss when to drop testnet3. I can rebase this for a while if we decide v30 is too early.
⚠️ Sjors opened an issue: "RFC: when to drop testnet3"
(https://github.com/bitcoin/bitcoin/issues/31975)
Testnet3 was deprecated in v28 with the introduction of testnet4. #31974 implements its removal.
Dropping it in v30 seems reasonable to me, but let's discuss this here as to not clutter the implementation PR.
@instagibbs wrote:
> I've said it elsewhere, but I think it deserves a quite long deprecation cycle, along with some data gathering.
>
> We don't want services not upgrading for fixes because their test environment would break.
>
> e.g., what exchanges with testnet environments have ad
...
(https://github.com/bitcoin/bitcoin/issues/31975)
Testnet3 was deprecated in v28 with the introduction of testnet4. #31974 implements its removal.
Dropping it in v30 seems reasonable to me, but let's discuss this here as to not clutter the implementation PR.
@instagibbs wrote:
> I've said it elsewhere, but I think it deserves a quite long deprecation cycle, along with some data gathering.
>
> We don't want services not upgrading for fixes because their test environment would break.
>
> e.g., what exchanges with testnet environments have ad
...
💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1977838755)
@marcofleon @dergoegge Do you have feedback on what your preferred approach would be here?
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1977838755)
@marcofleon @dergoegge Do you have feedback on what your preferred approach would be here?
💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2694989641)
PSA: Bitcoin Core PR Review Club will cover this PR in its next meeting at 2025-03-05 at 17:00 UTC. See https://bitcoincore.reviews/31405 for notes, questions, and instructions on [how to join](https://bitcoincore.reviews/).
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2694989641)
PSA: Bitcoin Core PR Review Club will cover this PR in its next meeting at 2025-03-05 at 17:00 UTC. See https://bitcoincore.reviews/31405 for notes, questions, and instructions on [how to join](https://bitcoincore.reviews/).
🤔 polespinasa reviewed a pull request: "Add mainnet assumeutxo param at height 880,000"
(https://github.com/bitcoin/bitcoin/pull/31969#pullrequestreview-2654759272)
ACK
Tested and got same values when dumping the utxo set
```
bitcoin-cli -rpcclienttimeout=0 -named dumptxoutset utxoset_880000.dat rollback=880000
{
"coins_written": 184821030,
"base_hash": "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880",
"base_height": 880000,
"path": "/home/sliv3r/.bitcoin/utxoset_880000.dat",
"txoutset_hash": "dbd190983eaf433ef7c15f78a278ae42c00ef52e0fd2a54953782175fbadcea9",
"nchaintx": 1145604538
}
```
Also got same sha256 fo
...
(https://github.com/bitcoin/bitcoin/pull/31969#pullrequestreview-2654759272)
ACK
Tested and got same values when dumping the utxo set
```
bitcoin-cli -rpcclienttimeout=0 -named dumptxoutset utxoset_880000.dat rollback=880000
{
"coins_written": 184821030,
"base_hash": "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880",
"base_height": 880000,
"path": "/home/sliv3r/.bitcoin/utxoset_880000.dat",
"txoutset_hash": "dbd190983eaf433ef7c15f78a278ae42c00ef52e0fd2a54953782175fbadcea9",
"nchaintx": 1145604538
}
```
Also got same sha256 fo
...
💬 jonatack commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977897446)
Oops -- done, thank you!
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977897446)
Oops -- done, thank you!
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977907287)
I posted the link above with example and I haven't seen a case where the RPC was hidden in there, so I think "the way it has always been" was to not deprecate. We haven't removed a whole RPC in a while though. But we can look at the options and returned values that were deprecated too: In that case it would make sense to remove the documentation of these options at the same time as we are deprecating them if we are following the justification for hiding. But instead we only add information that
...
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977907287)
I posted the link above with example and I haven't seen a case where the RPC was hidden in there, so I think "the way it has always been" was to not deprecate. We haven't removed a whole RPC in a while though. But we can look at the options and returned values that were deprecated too: In that case it would make sense to remove the documentation of these options at the same time as we are deprecating them if we are following the justification for hiding. But instead we only add information that
...
💬 jonatack commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977908894)
> We shouldn't have `|` at the end of the last clause, as this will make it match the empty string too (so effectively everything starting with `Satoshi:` matches).
Good catch. It looks like this would mean that the regex has been doing this since a8ec9eede4c745c6b6fd76974816ffad8034129a one year ago?
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977908894)
> We shouldn't have `|` at the end of the last clause, as this will make it match the empty string too (so effectively everything starting with `Satoshi:` matches).
Good catch. It looks like this would mean that the regex has been doing this since a8ec9eede4c745c6b6fd76974816ffad8034129a one year ago?
💬 jonatack commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977921907)
I don't recall deprecated RPCs being set to hidden in the past. Generally we do that for developer-only ones that could represent footguns for users. If this RPC is considered to be sufficiently dangerous, that could perhaps justify hiding it, but not due only to being deprecated.
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1977921907)
I don't recall deprecated RPCs being set to hidden in the past. Generally we do that for developer-only ones that could represent footguns for users. If this RPC is considered to be sufficiently dangerous, that could perhaps justify hiding it, but not due only to being deprecated.
💬 EthanHeilman commented on pull request "tests: improves tapscript unit tests":
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2695119699)
@dergoegge
I agree and I'd like to add those tests in a second PR if this PR is merged?
I don't have a lot of time this month and next to work on them and I worry about putting in more time for a PR that may not be merged.
(https://github.com/bitcoin/bitcoin/pull/31640#issuecomment-2695119699)
@dergoegge
I agree and I'd like to add those tests in a second PR if this PR is merged?
I don't have a lot of time this month and next to work on them and I worry about putting in more time for a PR that may not be merged.
💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1977960657)
I may have a situation in HTTP where SendBytes() is being called from a thread other than the Sockman I/O loop, which would be an "optimistic send" directly from a worker thread. `CConnman::PushMessage()` has similar logic, but since p2p doesn't use worker threads I don't think it would cause an issue there. I think this assertion may be the only real conflict for that. `m_connected_mutex` is only used in `GetConnectionSockets()` -- would a little lock-waiting there be so terrible?
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1977960657)
I may have a situation in HTTP where SendBytes() is being called from a thread other than the Sockman I/O loop, which would be an "optimistic send" directly from a worker thread. `CConnman::PushMessage()` has similar logic, but since p2p doesn't use worker threads I don't think it would cause an issue there. I think this assertion may be the only real conflict for that. `m_connected_mutex` is only used in `GetConnectionSockets()` -- would a little lock-waiting there be so terrible?
💬 laanwj commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977961350)
Yes, i didn't notice before ! It's too easy to make this mistake.
It also makes me wonder whether this is a useful filter at all, or whether to replace it with a simpler one that extracts the version number, followed by some minimum/maximum bounds check. But that's not something that needs to be addressed in this PR.
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977961350)
Yes, i didn't notice before ! It's too easy to make this mistake.
It also makes me wonder whether this is a useful filter at all, or whether to replace it with a simpler one that extracts the version number, followed by some minimum/maximum bounds check. But that's not something that needs to be addressed in this PR.
💬 jonatack commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977962385)
Not sure I see a difference in the makeseeds output. Done in the first commit attributed to you (thanks!)
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977962385)
Not sure I see a difference in the makeseeds output. Done in the first commit attributed to you (thanks!)
💬 laanwj commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977969707)
Thanks!
(https://github.com/bitcoin/bitcoin/pull/31960#discussion_r1977969707)
Thanks!