💬 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)
💬 Nimano1981 commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649390)
@hebasto
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649390)
@hebasto
💬 Nimano1981 commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649561)
#27191
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1513649561)
#27191
💬 theuni commented on pull request "depends: fix compiling bdb with clang-16 on aarch64":
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1513651367)
Just in case it's ever helpful (or as @MarcoFalke said, in case we need this for c++20), here's a set of depends patches for bdb that should fix the existing configure checks: https://github.com/theuni/bitcoin/commits/bdb-configure-int-checks
The code itself still isn't patched for the missing prototypes, but if the time comes and we need to do that, it should be simple enough.
(https://github.com/bitcoin/bitcoin/pull/27462#issuecomment-1513651367)
Just in case it's ever helpful (or as @MarcoFalke said, in case we need this for c++20), here's a set of depends patches for bdb that should fix the existing configure checks: https://github.com/theuni/bitcoin/commits/bdb-configure-int-checks
The code itself still isn't patched for the missing prototypes, but if the time comes and we need to do that, it should be simple enough.
💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170497181)
`config.cpp` still produces the <fstream> false positive. Is that acceptable?
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170497181)
`config.cpp` still produces the <fstream> false positive. Is that acceptable?
💬 fanquake commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714252)
cc @Emzy
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714252)
cc @Emzy
💬 jonatack commented on pull request "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714579)
Bringing out of draft after finishing the manual seeds updates and taking review feedback.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513714579)
Bringing out of draft after finishing the manual seeds updates and taking review feedback.
👋 jonatack's pull request is ready for review: "p2p: update manual tor/i2p/cjdns mainnet seeds for 25.x"
(https://github.com/bitcoin/bitcoin/pull/27488)
(https://github.com/bitcoin/bitcoin/pull/27488)
📝 Muhammedhamid23 opened a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ pinheadmz closed a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
(https://github.com/bitcoin/bitcoin/pull/27489)
💬 jonatack commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513731469)
> cc @Emzy
Yes. We checked in earlier today. Probably late evening now, pinged him over IRC to have a look.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513731469)
> cc @Emzy
Yes. We checked in earlier today. Probably late evening now, pinged him over IRC to have a look.
📝 fanquake locked a pull request: "2023"
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27489)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 Emzy commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513738537)
ACK.
Verified that my nodes are added.
They are long running. And I plan to run them for the foreseeable future.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513738537)
ACK.
Verified that my nodes are added.
They are long running. And I plan to run them for the foreseeable future.
💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1513771771)
re: https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1507916357
@ajtowns I like the safety check you suggested, and agree it could catch some programming errors. But I also think what `send` and `sendall` functions are trying to do to provide compatibility is reasonable, and adding the check would limit what they are able to do.
`send` and `sendall` currently accept `conf_target`, `estimate_mode` and `fee_rate` values as positional parameters (1), or as named parameters (2), or a
...
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1513771771)
re: https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1507916357
@ajtowns I like the safety check you suggested, and agree it could catch some programming errors. But I also think what `send` and `sendall` functions are trying to do to provide compatibility is reasonable, and adding the check would limit what they are able to do.
`send` and `sendall` currently accept `conf_target`, `estimate_mode` and `fee_rate` values as positional parameters (1), or as named parameters (2), or a
...
💬 achow101 commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513776313)
What criteria is used to choose the manually selected nodes?
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513776313)
What criteria is used to choose the manually selected nodes?