🤔 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
🤔 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.
(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.
✅ fanquake closed an issue: "doc: GUI Python dependency is not documented"
(https://github.com/bitcoin/bitcoin/issues/33146)
(https://github.com/bitcoin/bitcoin/issues/33146)
🚀 fanquake merged a pull request: "cmake: Drop python dependency for translate"
(https://github.com/bitcoin/bitcoin/pull/33209)
(https://github.com/bitcoin/bitcoin/pull/33209)
💬 baybaker11-lgtm commented on pull request "Fix compatibility with `-debuglogfile` command-line option":
(https://github.com/bitcoin/bitcoin/pull/33215#issuecomment-3200637134)
Drop the mouse
(https://github.com/bitcoin/bitcoin/pull/33215#issuecomment-3200637134)
Drop the mouse
💬 stickies-v commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200679164)
> 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.
Any test can slow down or hang for multiple reasons, that's why we have CI timeouts. I don't think it's pragmatic to use unexpected program flow to control timeouts.
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200679164)
> 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.
Any test can slow down or hang for multiple reasons, that's why we have CI timeouts. I don't think it's pragmatic to use unexpected program flow to control timeouts.
💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3200698339)
> @Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.
>
> I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output: self.test_sendrawtransaction() self.test_mixed_output_types()
>
> [test_sp_receive.patch](https://github.com/user-attachments/files/21834964/test_sp_receive.patch)
I think these might be more appropriate for the sending PR https://github.com/bitcoin/bit
...
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3200698339)
> @Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.
>
> I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output: self.test_sendrawtransaction() self.test_mixed_output_types()
>
> [test_sp_receive.patch](https://github.com/user-attachments/files/21834964/test_sp_receive.patch)
I think these might be more appropriate for the sending PR https://github.com/bitcoin/bit
...
💬 hebasto commented on pull request "Release: Prepare "Translation string freeze" step":
(https://github.com/bitcoin/bitcoin/pull/33193#issuecomment-3200791226)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/33193#issuecomment-3200791226)
Rebased.
💬 fanquake commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3200797532)
> This is tagged for 30.0, but feature freeze is in less than two weeks, and it still needs rebase, so it'll likely miss the milestone.
Feature freeze doesn't seem important here, given this isn't a feature; it's part of a cleanup of which the prior PRs have already landed, so landing this any time before branch off seems fine (assuming it gets rebased, have pinged @theuni).
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3200797532)
> This is tagged for 30.0, but feature freeze is in less than two weeks, and it still needs rebase, so it'll likely miss the milestone.
Feature freeze doesn't seem important here, given this isn't a feature; it's part of a cleanup of which the prior PRs have already landed, so landing this any time before branch off seems fine (assuming it gets rebased, have pinged @theuni).
💬 hebasto commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2285302380)
Translations followup in https://github.com/bitcoin/bitcoin/pull/33193.
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2285302380)
Translations followup in https://github.com/bitcoin/bitcoin/pull/33193.
💬 maflcko commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200809745)
> 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.
The timeout is purely virtual and never waited on or synced on in real wall-clock time, so I don't think this comment applies?
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200809745)
> 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.
The timeout is purely virtual and never waited on or synced on in real wall-clock time, so I don't think this comment applies?