📝 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.
💬 hebasto commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3200322167)
> Incorporated into [bitcoin/bitcoin#33215](https://github.com/bitcoin/bitcoin/pull/33215).
That PR is not actually ready.
> Closing.
Reopened.
(https://github.com/bitcoin-core/gui/pull/884#issuecomment-3200322167)
> Incorporated into [bitcoin/bitcoin#33215](https://github.com/bitcoin/bitcoin/pull/33215).
That PR is not actually ready.
> Closing.
Reopened.
💬 stickies-v commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3200395197)
Rebased to address merge conflict from #32896, no other changes.
(https://github.com/bitcoin/bitcoin/pull/32983#issuecomment-3200395197)
Rebased to address merge conflict from #32896, no other changes.
💬 Henry3461 commented on issue "wallet: control which taproot script path to spend":
(https://github.com/bitcoin/bitcoin/issues/33084#issuecomment-3200404674)
Hello folks, an online seminar referenced a platform demo so I clicked Summit Edge Platform https://summit-edge.org/ and explored the interface live. In Canada the linked documentation and in-app tooltips made learning faster than usual; my roommate peeked over and praised the tidy layout. I set up watchlists, configured alerts, and tested a conservative signal. Follow-up emails were clear and polite, and this balanced first impression made me want to compare a few strategies with my partner ove
...
(https://github.com/bitcoin/bitcoin/issues/33084#issuecomment-3200404674)
Hello folks, an online seminar referenced a platform demo so I clicked Summit Edge Platform https://summit-edge.org/ and explored the interface live. In Canada the linked documentation and in-app tooltips made learning faster than usual; my roommate peeked over and praised the tidy layout. I set up watchlists, configured alerts, and tested a conservative signal. Follow-up emails were clear and polite, and this balanced first impression made me want to compare a few strategies with my partner ove
...
💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3200417035)
Rebased 10c3a0689a3b73eb2b291ee81dd85066fa32497d -> 3843a15f5b5b50d47c2524f607ba0b01c90d42b1 ([spendblock_9](https://github.com/TheCharlatan/bitcoin/tree/spendblock_9) -> [spendblock_10](https://github.com/TheCharlatan/bitcoin/tree/spendblock_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_9..spendblock_10))
* Fixed merge conflict with #33105
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3200417035)
Rebased 10c3a0689a3b73eb2b291ee81dd85066fa32497d -> 3843a15f5b5b50d47c2524f607ba0b01c90d42b1 ([spendblock_9](https://github.com/TheCharlatan/bitcoin/tree/spendblock_9) -> [spendblock_10](https://github.com/TheCharlatan/bitcoin/tree/spendblock_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_9..spendblock_10))
* Fixed merge conflict with #33105