👍 hebasto approved a pull request: "depends: reuse _config_opts for CMake options"
(https://github.com/bitcoin/bitcoin/pull/27496#pullrequestreview-1392993005)
ACK 63c0c4ff10b2f6ed8510ef372a5b1f6a6494b179
This change allows to reuse this machinery: https://github.com/bitcoin/bitcoin/blob/d26a71a94ac4ae1b1a091f4412d390afba69b2f8/depends/funcs.mk#L128-L131
(https://github.com/bitcoin/bitcoin/pull/27496#pullrequestreview-1392993005)
ACK 63c0c4ff10b2f6ed8510ef372a5b1f6a6494b179
This change allows to reuse this machinery: https://github.com/bitcoin/bitcoin/blob/d26a71a94ac4ae1b1a091f4412d390afba69b2f8/depends/funcs.mk#L128-L131
💬 jonatack commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1515510969)
> I think it's great that we now have a few more tor seeds than 11, which seemed rather low to me considering the large number of bitcoin nodes reachable via tor.
Credit to you, @mzumsande, for suggesting to me yesterday that we add more.
I've repushed after pulling in 3cc989da5c750e740705131bed05bbf93bfdf169 from #26681 and re-running the steps in `contrib/seeds/README.md`.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1515510969)
> I think it's great that we now have a few more tor seeds than 11, which seemed rather low to me considering the large number of bitcoin nodes reachable via tor.
Credit to you, @mzumsande, for suggesting to me yesterday that we add more.
I've repushed after pulling in 3cc989da5c750e740705131bed05bbf93bfdf169 from #26681 and re-running the steps in `contrib/seeds/README.md`.
💬 yusufsahinhamza commented on pull request "net, contrib: Bugfix for checking bad dns seeds without casting in `makeseeds.py`":
(https://github.com/bitcoin/bitcoin/pull/26681#discussion_r1171947351)
Yes, i've meant to ignore the first line (the comment for column names) in `seeds_main.txt` by putting the **"#"** conditional.
@jonatack I've thought about comparing it in **str** but then decided to cast with **int** because, seems like the `good` column meant to be **0 and 1** which is **integer**, so i rather use casting and have the **"#"** conditional.
(https://github.com/bitcoin/bitcoin/pull/26681#discussion_r1171947351)
Yes, i've meant to ignore the first line (the comment for column names) in `seeds_main.txt` by putting the **"#"** conditional.
@jonatack I've thought about comparing it in **str** but then decided to cast with **int** because, seems like the `good` column meant to be **0 and 1** which is **integer**, so i rather use casting and have the **"#"** conditional.
💬 jonatack commented on pull request "net, contrib: Bugfix for checking bad dns seeds without casting in `makeseeds.py`":
(https://github.com/bitcoin/bitcoin/pull/26681#discussion_r1171950558)
@yusufsahinhamza I cherry-picked your commit here as-is into #27488 to start using it for generating the hardcoded seeds in the v25 release.
(https://github.com/bitcoin/bitcoin/pull/26681#discussion_r1171950558)
@yusufsahinhamza I cherry-picked your commit here as-is into #27488 to start using it for generating the hardcoded seeds in the v25 release.
💬 theStack commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171962843)
tiny nit: could be put in the "private:" section, since there is a static constexpr `size()` function returning this value below anyway.
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171962843)
tiny nit: could be put in the "private:" section, since there is a static constexpr `size()` function returning this value below anyway.
💬 theStack commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171972333)
There's a theoretical possibility that this call crashes due to failed assertion `assert(fValid)`, if the generated key from the previous round (=first 32 bytes of ellswift encoded pubkey) was not valid. Due to the practical low probably for this to ever happen (if my math isn't off, on average for every 2^(127,65) randomly generated key is invalid), I guess it's not worth it to add any checks here, especially since it's only a benchmark?
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171972333)
There's a theoretical possibility that this call crashes due to failed assertion `assert(fValid)`, if the generated key from the previous round (=first 32 bytes of ellswift encoded pubkey) was not valid. Due to the practical low probably for this to ever happen (if my math isn't off, on average for every 2^(127,65) randomly generated key is invalid), I guess it's not worth it to add any checks here, especially since it's only a benchmark?
💬 jonatack commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1515600786)
Again here https://cirrus-ci.com/task/5250935128064000?logs=ci#L3275
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1515600786)
Again here https://cirrus-ci.com/task/5250935128064000?logs=ci#L3275
⚠️ ariard opened an issue: "Document CoreDev organization"
(https://github.com/bitcoin/bitcoin/issues/27497)
### Please describe the feature you'd like to see added.
They have been attempts and discussions in the past to document other aspects of project culture such as maintenance with #25560. The organization of in-person events can be itself a source of contention, and I'm speaking here as a former
organizer. While it's voluntary like all other tasks in Bitcoin open-source, there is a lot of subjectivity in the choice of schedule, place and the selection of attendees. Documenting such process will
...
(https://github.com/bitcoin/bitcoin/issues/27497)
### Please describe the feature you'd like to see added.
They have been attempts and discussions in the past to document other aspects of project culture such as maintenance with #25560. The organization of in-person events can be itself a source of contention, and I'm speaking here as a former
organizer. While it's voluntary like all other tasks in Bitcoin open-source, there is a lot of subjectivity in the choice of schedule, place and the selection of attendees. Documenting such process will
...
💬 ajtowns commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172071849)
I think this should be:
```c++
if (!inner.m_opts.also_positional) {
CHECK_NONFATAL(named_args.insert(inner_name).second);
}
```
Otherwise a duplicate positional parameter after the `OBJ_NAMED_PARAMS` entry will give an error.
(https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172071849)
I think this should be:
```c++
if (!inner.m_opts.also_positional) {
CHECK_NONFATAL(named_args.insert(inner_name).second);
}
```
Otherwise a duplicate positional parameter after the `OBJ_NAMED_PARAMS` entry will give an error.
🤔 ajtowns reviewed a pull request: "refactor: Move chain constants to the util library"
(https://github.com/bitcoin/bitcoin/pull/27491#pullrequestreview-1393199662)
Would be better if the second commit (renaming `CBaseChainParam::MAIN` to `ChainType::MAIN` etc) could be a scripted-diff. Perhaps structure it as:
* introduce ChainType
* add `CreateBaseChainParams(ChainType)` and replace `CreateBaseChainParams(string)` with `{ auto ct = ChainTypeFromString(chain); if (ct) { return CreateBaseChainParams(ct); } else { throw...; }` (and so on)
* scripted-diff to switch to `ChainType::MAIN` and `args.GetChainType()`
* remove the now unused functions that
...
(https://github.com/bitcoin/bitcoin/pull/27491#pullrequestreview-1393199662)
Would be better if the second commit (renaming `CBaseChainParam::MAIN` to `ChainType::MAIN` etc) could be a scripted-diff. Perhaps structure it as:
* introduce ChainType
* add `CreateBaseChainParams(ChainType)` and replace `CreateBaseChainParams(string)` with `{ auto ct = ChainTypeFromString(chain); if (ct) { return CreateBaseChainParams(ct); } else { throw...; }` (and so on)
* scripted-diff to switch to `ChainType::MAIN` and `args.GetChainType()`
* remove the now unused functions that
...
💬 ajtowns commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1172082054)
Calling it `ChainName(ChainType chain)` might be a bit clearer?
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1172082054)
Calling it `ChainName(ChainType chain)` might be a bit clearer?
💬 ajtowns commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1172076982)
Put `MAIN` on a newline and just use 4-space indentation? Newlines for `{` and `}` as well, per style guide? Trailing `,` after `REGTEST` in case we add any new ones later?
(https://github.com/bitcoin/bitcoin/pull/27491#discussion_r1172076982)
Put `MAIN` on a newline and just use 4-space indentation? Newlines for `{` and `}` as well, per style guide? Trailing `,` after `REGTEST` in case we add any new ones later?
💬 fanquake commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1515959787)
> Building them with CMake will allow us to drop several deps that we currently have (autoconf, automake, pkg-config, etc) which would be unfortunate to carry over after the switch-over.
I know it's not relevant for this PR, but is the plan to upstream CMake support everywhere it's missing (sqlite, all the qt deps etc), and maintain it ourselves where it's not upstreamable (i.e bdb), otherwise we still can't drop any deps.
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1515959787)
> Building them with CMake will allow us to drop several deps that we currently have (autoconf, automake, pkg-config, etc) which would be unfortunate to carry over after the switch-over.
I know it's not relevant for this PR, but is the plan to upstream CMake support everywhere it's missing (sqlite, all the qt deps etc), and maintain it ourselves where it's not upstreamable (i.e bdb), otherwise we still can't drop any deps.
🚀 fanquake merged a pull request: "contrib: Bugfix for checking bad dns seeds without casting in `makeseeds.py`"
(https://github.com/bitcoin/bitcoin/pull/26681)
(https://github.com/bitcoin/bitcoin/pull/26681)
💬 fanquake commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1515982170)
Needs rebase
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1515982170)
Needs rebase
💬 bavr2020 commented on pull request "Register `wallet::AddressPurpose` type":
(https://github.com/bitcoin-core/gui/pull/726#issuecomment-1515987477)
****
(https://github.com/bitcoin-core/gui/pull/726#issuecomment-1515987477)
****
📝 fanquake locked a pull request: "Register `wallet::AddressPurpose` type"
(https://github.com/bitcoin-core/gui/pull/726)
This PR is a follow up of bitcoin/bitcoin#27217.
Fixes #725.
(https://github.com/bitcoin-core/gui/pull/726)
This PR is a follow up of bitcoin/bitcoin#27217.
Fixes #725.
💬 MarcoFalke commented on issue "test: use-of-uninitialized-value in sqlite3Strlen30":
(https://github.com/bitcoin/bitcoin/issues/27222#issuecomment-1516009197)
Too bad this doesn't easily reproduce outside of the CI env with valgrind. I guess if someone wants to take a look, it might be easiest to fiddle inside the CI env interactively.
(https://github.com/bitcoin/bitcoin/issues/27222#issuecomment-1516009197)
Too bad this doesn't easily reproduce outside of the CI env with valgrind. I guess if someone wants to take a look, it might be easiest to fiddle inside the CI env interactively.
💬 glozow commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1516052330)
linking #9923
perhaps also relevant: #10594, #10051
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1516052330)
linking #9923
perhaps also relevant: #10594, #10051
💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1516063771)
Gave this another look. No luck with `rr` and haven't been able to reproduce this locally or in the CI.
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1516063771)
Gave this another look. No luck with `rr` and haven't been able to reproduce this locally or in the CI.