💬 achow101 commented on pull request "[24.1] Bump version to 24.1rc2":
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1513478139)
ACK 03a16a1da09f45721c799c719e48350dfe12ae88
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1513478139)
ACK 03a16a1da09f45721c799c719e48350dfe12ae88
💬 josibake commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1513518396)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
verified that I got all the same values on my node for mainnet
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1513518396)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
verified that I got all the same values on my node for mainnet
📝 jonatack opened a pull request: "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x"
(https://github.com/bitcoin/bitcoin/pull/27488)
Update the hardcoded P2P network seeds for 25.x after updating the manual seeds (we needed more Tor and CJDNS seeds, in particular) and the generation script as necessary. Previous update was #25853.
Can be tested by following the steps in `contrib/seeds/README.md`.
Tool output:
```
$ python3 makeseeds.py -a asmap-filled.dat -s seeds_main.txt > nodes_main.txt
Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
IPv4 IPv6 Onion Pass
...
(https://github.com/bitcoin/bitcoin/pull/27488)
Update the hardcoded P2P network seeds for 25.x after updating the manual seeds (we needed more Tor and CJDNS seeds, in particular) and the generation script as necessary. Previous update was #25853.
Can be tested by following the steps in `contrib/seeds/README.md`.
Tool output:
```
$ python3 makeseeds.py -a asmap-filled.dat -s seeds_main.txt > nodes_main.txt
Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
IPv4 IPv6 Onion Pass
...
💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513531812)
Briefly in draft while I run a script to update more manual seeds for reachability/uptime/service bit 1.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513531812)
Briefly in draft while I run a script to update more manual seeds for reachability/uptime/service bit 1.
🤔 pablomartin4btc reviewed a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1390605733)
Concept ACK. Light code review, I'd to spend more time with it, which I'll do soon. Good stuff!
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1390605733)
Concept ACK. Light code review, I'd to spend more time with it, which I'll do soon. Good stuff!
💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170366895)
Manually bumping dates in copyright headers is never required
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170366895)
Manually bumping dates in copyright headers is never required
💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513555787)
Updated de2546e672d0a2103228d777b35bfa6aab4881ee -> ec51c252de42e11906f114ac05e476fd88ca2176 ([splitSystemArgs_2](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_2) -> [splitSystemArgs_3](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_2..splitSystemArgs_3)):
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1374037220) by adding a config
...
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513555787)
Updated de2546e672d0a2103228d777b35bfa6aab4881ee -> ec51c252de42e11906f114ac05e476fd88ca2176 ([splitSystemArgs_2](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_2) -> [splitSystemArgs_3](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_2..splitSystemArgs_3)):
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1374037220) by adding a config
...
💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170370860)
Will drop in next push, thanks.
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170370860)
Will drop in next push, thanks.
💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170376722)
I think this can also just be left as-is. "After removing duplicates" certainly fits the tone/amongst the other sentences better. Not really sure why it needs changing.
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170376722)
I think this can also just be left as-is. "After removing duplicates" certainly fits the tone/amongst the other sentences better. Not really sure why it needs changing.
💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170387609)
Every other sentence takes the form of a verb followed by an action. I don't mind dropping that, but it seems more in line with the other printed lines.
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170387609)
Every other sentence takes the form of a verb followed by an action. I don't mind dropping that, but it seems more in line with the other printed lines.
💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170390258)
Skip/remove/enforce/require etc.
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170390258)
Skip/remove/enforce/require etc.
💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170392722)
I don't really mind, I just don't see why there's multiple unrelated changes in this PR (manual copyright header bumping, rewriting log output etc), when it could just be a straight-forward seed update.
(https://github.com/bitcoin/bitcoin/pull/27488#discussion_r1170392722)
I don't really mind, I just don't see why there's multiple unrelated changes in this PR (manual copyright header bumping, rewriting log output etc), when it could just be a straight-forward seed update.
👍 ryanofsky approved a pull request: "refactor: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1390630091)
Code review ACK ec51c252de42e11906f114ac05e476fd88ca2176. Hoping this moveonly change could be reviewed and merged quickly since it conflicts with various PR's I have open
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1390630091)
Code review ACK ec51c252de42e11906f114ac05e476fd88ca2176. Hoping this moveonly change could be reviewed and merged quickly since it conflicts with various PR's I have open
💬 ryanofsky commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170374206)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)
Any reason these header are only included if WIN32 is not defined? I know the ifndef is preexisting, but it seems like it would make more sense to make these includes unconditional.
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170374206)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)
Any reason these header are only included if WIN32 is not defined? I know the ifndef is preexisting, but it seems like it would make more sense to make these includes unconditional.
💬 ryanofsky commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170386744)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)
Seems like extra whitespace this line.
Note for reviewers: This function previously wasn't exposed as part of the header, but was moved here as part of splitting up args and config file. It can go away with a followup change making config parsing functions more independent.
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170386744)
In commit "move-only: Extract common/args and common/config.cpp from util/system" (ec51c252de42e11906f114ac05e476fd88ca2176)
Seems like extra whitespace this line.
Note for reviewers: This function previously wasn't exposed as part of the header, but was moved here as part of splitting up args and config file. It can go away with a followup change making config parsing functions more independent.
💬 fanquake commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1513602373)
> What are possible drawbacks here?
I don't really understand adding it given [your comment above](https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1494267607):
> I'd avoid it as it will change code paths being parsed now
You suggested avoiding changing the code paths being parsed, and then suggested adding a define that changes the code paths being parsed?
In its current state, this PR is adding a Boost define, because it has a side-effect that basically "tricks" `clang-ti
...
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1513602373)
> What are possible drawbacks here?
I don't really understand adding it given [your comment above](https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1494267607):
> I'd avoid it as it will change code paths being parsed now
You suggested avoiding changing the code paths being parsed, and then suggested adding a define that changes the code paths being parsed?
In its current state, this PR is adding a Boost define, because it has a side-effect that basically "tricks" `clang-ti
...
💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170434860)
I checked it and its history again and can't think of a good reason either. Should I amend here, or go for the follow up?
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170434860)
I checked it and its history again and can't think of a good reason either. Should I amend here, or go for the follow up?
💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1170436504)
done, although I think we should assert `nAddSize < max_blockfile_size;` - see explanation above.
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1170436504)
done, although I think we should assert `nAddSize < max_blockfile_size;` - see explanation above.
💬 mzumsande commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1513632824)
Thanks for the reviews! I pushed an update to address the outstanding comments and added the test by @pinheadmz (thanks!).
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1513632824)
Thanks for the reviews! I pushed an update to address the outstanding comments and added the test by @pinheadmz (thanks!).
💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170443781)
```suggestion
" src/common/args.cpp"\
" src/common/config.cpp"\
```
nit (if you retouch)
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170443781)
```suggestion
" src/common/args.cpp"\
" src/common/config.cpp"\
```
nit (if you retouch)