Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 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).
🤔 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
...
💬 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.
💬 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.
👍 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
...
📝 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.
hebasto closed a pull request: "Fix compatibility with `-debuglogfile` command-line option"
(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
🤔 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.
🤔 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
...
💬 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
...
💬 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"
📝 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.
💬 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).
📝 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.
💬 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.
💬 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.
💬 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
...
💬 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
🤔 janb84 reviewed a pull request: "test: modify logging_filesize_rate_limit params"
(https://github.com/bitcoin/bitcoin/pull/33211#pullrequestreview-3131994160)
approach ACK ab5c3be4b384d48d514b06dc8a391f1efa51ea38

PR increases the test rest window to 1 hour from 20 seconds, to accommodate slow test execution. And reduces test log.

Small worry is by increasing the test rest window to 1h is that this will impact CI if a test will hang for some reason, it is a big increase.