π€ BrandonOdiwuor reviewed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131564457)
I still don't understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
(https://github.com/bitcoin/bitcoin/pull/33190#pullrequestreview-3131564457)
I still don't understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
β
glozow closed an issue: "Add support for creating v3 raw transactions in `createrawtransaction` RPC"
(https://github.com/bitcoin/bitcoin/issues/31348)
(https://github.com/bitcoin/bitcoin/issues/31348)
π glozow merged a pull request: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
(https://github.com/bitcoin/bitcoin/pull/32896)
π¬ BrandonOdiwuor commented on pull request "doc: gen-manpages.py should check build options":
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3200113920)
@janb84 thanks for having a look
I think introducing a main function would require a huge refactor to the file for such small check.
I would also love to hear some advantages of adding a main function here
(https://github.com/bitcoin/bitcoin/pull/33085#issuecomment-3200113920)
@janb84 thanks for having a look
I think introducing a main function would require a huge refactor to the file for such small check.
I would also love to hear some advantages of adding a main function here
π¬ ismaelsadeeq commented on pull request "net: Provide block templates to peers on request":
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3200160757)
Nice idea.
A couple of questions:
1. How do nodes build the block template they share with peers? When they use only their mempool to build those templates, I donβt think that the transactions with diverging mempool policy will relay through the network and improve the compact block reconstruction process via this mechanism. Only peers whose outbound connections are miners will benefit.
2. We donβt accept transactions in the mempool that have divergent policy because we deem them non-st
...
(https://github.com/bitcoin/bitcoin/pull/33191#issuecomment-3200160757)
Nice idea.
A couple of questions:
1. How do nodes build the block template they share with peers? When they use only their mempool to build those templates, I donβt think that the transactions with diverging mempool policy will relay through the network and improve the compact block reconstruction process via this mechanism. Only peers whose outbound connections are miners will benefit.
2. We donβt accept transactions in the mempool that have divergent policy because we deem them non-st
...
π hebasto opened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
π¬ sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284831733)
This was with an earlier version of the PR which called `load_capnp_modules` from `set_test_params` (which tried to do so before deciding whether pycapnp was even installed).
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284831733)
This was with an earlier version of the PR which called `load_capnp_modules` from `set_test_params` (which tried to do so before deciding whether pycapnp was even installed).
π€ hodlinator reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3131691825)
Latest push:
* Breaks up test into multiple boost test cases instead of regular functions (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2277678459). In doing so, I changed back to using a custom fixture as on master. Diffing with dimmed-zebra helps show what was just moved and what actually changed `git diff 342359f 53341ea`.
* Removed redundant `assert()` after bringing that block closer to constants (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2271294627).
* Minor
...
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3131691825)
Latest push:
* Breaks up test into multiple boost test cases instead of regular functions (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2277678459). In doing so, I changed back to using a custom fixture as on master. Diffing with dimmed-zebra helps show what was just moved and what actually changed `git diff 342359f 53341ea`.
* Removed redundant `assert()` after bringing that block closer to constants (https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2271294627).
* Minor
...
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284849835)
Pushed version with multiple test cases.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284849835)
Pushed version with multiple test cases.
π¬ hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284838171)
Moved up the macro above the constants in the latest push, and brought the assert block up closer to the constants - in the process removing the final assert.
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2284838171)
Moved up the macro above the constants in the latest push, and brought the assert block up closer to the constants - in the process removing the final assert.
π stickies-v approved a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3131772733)
ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38
> then we could set set the reset window to 5 hours
That would require more changes to `CScheduler` because `MockForward` is currently [capped at 1h](https://github.com/bitcoin/bitcoin/blob/be356fc49be4a32f790c38ca72c76e9d6fa2de3d/src/scheduler.cpp#L82).
---
I considered previously to also add `log_lines` to the boost test context, but decided not to for brevity, but it would've been helpful in debugging this. I think the below diff would
...
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3131772733)
ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38
> then we could set set the reset window to 5 hours
That would require more changes to `CScheduler` because `MockForward` is currently [capped at 1h](https://github.com/bitcoin/bitcoin/blob/be356fc49be4a32f790c38ca72c76e9d6fa2de3d/src/scheduler.cpp#L82).
---
I considered previously to also add `log_lines` to the boost test context, but decided not to for brevity, but it would've been helpful in debugging this. I think the below diff would
...
π hebasto opened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin/bitcoin/pull/33215)
This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
(https://github.com/bitcoin/bitcoin/pull/33215)
This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
β
hebasto closed a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
(https://github.com/bitcoin-core/gui/pull/884)
π¬ hebasto commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3200273773)
Incorporated into https://github.com/bitcoin/bitcoin/pull/33215.
Closing
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3200273773)
Incorporated into https://github.com/bitcoin/bitcoin/pull/33215.
Closing
π€ stickies-v reviewed a pull request: "rpc: require integer verbosity; remove boolean 'verbose'"
(https://github.com/bitcoin/bitcoin/pull/33214#pullrequestreview-3131799623)
Directionally, this is the right way to go, but I don't think we should be breaking the API for just this change. Let's do these cleanups when we can bundle them with other, necessary breaking changes, on a case-by-case basis. Concept NACK.
(https://github.com/bitcoin/bitcoin/pull/33214#pullrequestreview-3131799623)
Directionally, this is the right way to go, but I don't think we should be breaking the API for just this change. Let's do these cleanups when we can bundle them with other, necessary breaking changes, on a case-by-case basis. Concept NACK.
π€ janb84 reviewed a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131829788)
re ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8
changes since last ACK:
- multilines are now joint
- changes in sorting
Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:
<details>
````C++
QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data l
...
(https://github.com/bitcoin/bitcoin/pull/33209#pullrequestreview-3131829788)
re ACK 3c4a109aa821cbf1e46a67275b4f456673ed13d8
changes since last ACK:
- multilines are now joint
- changes in sorting
Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:
<details>
````C++
QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data l
...
π¬ hebasto commented on pull request "cmake: Drop python dependency for translate":
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3200309116)
> Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:
>
> ```c++
> QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data loss."),
> QT_TRANSLATE_NOOP("bitcoin-core", "default wallet"),
> ```
This is expected. See: https://github.com/bitcoin/bitcoin/p
...
(https://github.com/bitcoin/bitcoin/pull/33209#issuecomment-3200309116)
> Manually checked diff of `bitcoinstrings.cpp` because of change of sorting, all the lines are included. This PR introduces 2 new lines in `bitcoinstrings.cpp`:
>
> ```c++
> QT_TRANSLATE_NOOP("bitcoin-core", "The %s path uses exFAT, which is known to have intermittent corruption problems on macOS. Move this directory to a different filesystem to avoid data loss."),
> QT_TRANSLATE_NOOP("bitcoin-core", "default wallet"),
> ```
This is expected. See: https://github.com/bitcoin/bitcoin/p
...
π¬ Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284919848)
Ah yes, now it's failing with a very cryptic "Tests successful"
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2284919848)
Ah yes, now it's failing with a very cryptic "Tests successful"
π hebasto converted_to_draft a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin/bitcoin/pull/33215)
This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
(https://github.com/bitcoin/bitcoin/pull/33215)
This PR avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
π¬ rebroad commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3200321352)
Why not simply evict nodes that are using up a lot of data that isn't resulting in mempool entries? Perhaps even ban nodes that keep doing this? (And perhaps also the ban logic to include a probationary period causing the ban duration to be extended for repeated offenders).
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3200321352)
Why not simply evict nodes that are using up a lot of data that isn't resulting in mempool entries? Perhaps even ban nodes that keep doing this? (And perhaps also the ban logic to include a probationary period causing the ban duration to be extended for repeated offenders).
π hebasto reopened a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.
(https://github.com/bitcoin-core/gui/pull/884)
This change avoids hardcoding the "debug.log" filename, ensuring compatibility with custom filenames provided via the `-debuglogfile` command-line option.