π¬ hebasto commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3481469144)
Add https://github.com/bitcoin-core/gui/pull/901?
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3481469144)
Add https://github.com/bitcoin-core/gui/pull/901?
π¬ purpleKarrot commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3481472896)
@maflcko, I get that checking for overflow after the cause is not going to work.
But if we have a buggy calculation that may overflow u32, changing the type to u64 is not a proper fix. What if it overflows u64? How do you want to detect that?
Would you agree that, if the calculation is fixed to detect overflow before the cause then using `size_t` would not be a problem at all?
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3481472896)
@maflcko, I get that checking for overflow after the cause is not going to work.
But if we have a buggy calculation that may overflow u32, changing the type to u64 is not a proper fix. What if it overflows u64? How do you want to detect that?
Would you agree that, if the calculation is fixed to detect overflow before the cause then using `size_t` would not be a problem at all?
π¬ hebasto commented on pull request "Modernize custom filtering":
(https://github.com/bitcoin-core/gui/pull/899#discussion_r2487168803)
Thanks! Fixed.
(https://github.com/bitcoin-core/gui/pull/899#discussion_r2487168803)
Thanks! Fixed.
π¬ stickies-v commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2487174964)
http://github.com/bitcoin/bitcoin/pull/32604/commits/d541409a64c60d127ff912dad9dea949d45dbd8c#diff-44fd50b51e8fc6799d38f193237fb921ec9d34306c448f64837524a17bac06eeR471-R472 introduced prefixing all messages with `[*]` to indicate that ratelimiting is currently being applied.
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2487174964)
http://github.com/bitcoin/bitcoin/pull/32604/commits/d541409a64c60d127ff912dad9dea949d45dbd8c#diff-44fd50b51e8fc6799d38f193237fb921ec9d34306c448f64837524a17bac06eeR471-R472 introduced prefixing all messages with `[*]` to indicate that ratelimiting is currently being applied.
π¬ hebasto commented on pull request "Remove HD seed reference from blank wallet tooltip":
(https://github.com/bitcoin-core/gui/pull/908#issuecomment-3481506281)
cc @achow101
(https://github.com/bitcoin-core/gui/pull/908#issuecomment-3481506281)
cc @achow101
π¬ 0xB10C commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/33680#discussion_r2487213289)
I plan have a closer look.
I don't use the utxocache tracepoints at the moment.
(https://github.com/bitcoin/bitcoin/pull/33680#discussion_r2487213289)
I plan have a closer look.
I don't use the utxocache tracepoints at the moment.
π¬ yancyribbens commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2487224418)
> SRD does NOT minimize the number of UTXOs included in the result. It only ensures that the selection adheres to the weight limit. Minimizing would imply that the selection is further pruned beyond adhering to the weight limit.
Your edit's make sense, thanks for the clarification. However, I thought it would be helpful to note _why_ the OutputGroups with the lowest `effective_value` are removed when the weight is exceeded. This seems at first counter-intuitive since it's not removing Outp
...
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2487224418)
> SRD does NOT minimize the number of UTXOs included in the result. It only ensures that the selection adheres to the weight limit. Minimizing would imply that the selection is further pruned beyond adhering to the weight limit.
Your edit's make sense, thanks for the clarification. However, I thought it would be helpful to note _why_ the OutputGroups with the lowest `effective_value` are removed when the weight is exceeded. This seems at first counter-intuitive since it's not removing Outp
...
π hebasto approved a pull request: "ci: Fix lint runner selection (and docker cache)"
(https://github.com/bitcoin/bitcoin/pull/33744#pullrequestreview-3412060705)
ACK 7632e0ba312a372259897c68fd7c7eb723df3738.
(https://github.com/bitcoin/bitcoin/pull/33744#pullrequestreview-3412060705)
ACK 7632e0ba312a372259897c68fd7c7eb723df3738.
π¬ hebasto commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487242811)
The GHA [docs](https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax) do not mention `inputs.<input_id>.options` at all.
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487242811)
The GHA [docs](https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax) do not mention `inputs.<input_id>.options` at all.
π hebasto approved a pull request: "random: scope environ extern to macOS, BSDs and Illumos"
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3412082427)
re-ACK 79d6f458e2300e1f47b94467cda233e1c761f8be.
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3412082427)
re-ACK 79d6f458e2300e1f47b94467cda233e1c761f8be.
π¬ willcl-ark commented on pull request "ci: Fix lint runner selection (and docker cache)":
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487261226)
Yep, I think the original implementation got wires crossed with workflow dispatch inputs https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-of-onworkflow_dispatchinputs
(https://github.com/bitcoin/bitcoin/pull/33744#discussion_r2487261226)
Yep, I think the original implementation got wires crossed with workflow dispatch inputs https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-of-onworkflow_dispatchinputs
π¬ yancyribbens commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2487266295)
This edit is a tautology. No matter what the change budgeting is, the change amount will be at least `CHANGE_LOWER` since this value is added to `CHANGE_LOWER`. Maybe it's not the place to explain to users of the API _why_ it matters that `CHANGE_LOWER` may not be enough of a change budget, although I would have no idea why without reading the commit that added this parameter.
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2487266295)
This edit is a tautology. No matter what the change budgeting is, the change amount will be at least `CHANGE_LOWER` since this value is added to `CHANGE_LOWER`. Maybe it's not the place to explain to users of the API _why_ it matters that `CHANGE_LOWER` may not be enough of a change budget, although I would have no idea why without reading the commit that added this parameter.
π¬ hebasto commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481668679)
> Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
cc @GBKS @johnny9
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481668679)
> Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
cc @GBKS @johnny9
π¬ fanquake commented on pull request "feat: shell completions for bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481668738)
Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481668738)
Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.
π¬ purpleKarrot commented on pull request "refactor: C++20 operators":
(https://github.com/bitcoin/bitcoin/pull/33771#discussion_r2487296852)
Thanks @stickies-v.
I am not convinced about `std::strong_ordering`.
`constexpr` will definitely be added as a follow up, together with `= default` where appropriate.
(https://github.com/bitcoin/bitcoin/pull/33771#discussion_r2487296852)
Thanks @stickies-v.
I am not convinced about `std::strong_ordering`.
`constexpr` will definitely be added as a follow up, together with `= default` where appropriate.
π¬ fanquake commented on pull request "depends: disable variables, rules and suffixes.":
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3481696397)
Guix Build (aarch64):
```bash
e0edc03339b5b32705b8eacbc213a6fd6e5b787caba574671bb3c18340a141ec guix-build-52b1595850f6/output/aarch64-linux-gnu/SHA256SUMS.part
55778a17e64cabdb200679240fa004b5c5c9a684ecae773e8497997ab7c968d6 guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu-debug.tar.gz
6a2f49dbc8238438d40d46c3ce699814ae1d9eb6d74dd0b4ff273db6648b181c guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu.tar.gz
2fce92
...
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3481696397)
Guix Build (aarch64):
```bash
e0edc03339b5b32705b8eacbc213a6fd6e5b787caba574671bb3c18340a141ec guix-build-52b1595850f6/output/aarch64-linux-gnu/SHA256SUMS.part
55778a17e64cabdb200679240fa004b5c5c9a684ecae773e8497997ab7c968d6 guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu-debug.tar.gz
6a2f49dbc8238438d40d46c3ce699814ae1d9eb6d74dd0b4ff273db6648b181c guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu.tar.gz
2fce92
...
π¬ johnny9 commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481702959)
> > Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
>
> cc [@GBKS](https://github.com/GBKS) [@johnny9](https://github.com/johnny9)
My vote would be to continue to add features to the Qt Widgets gui. There might be differing opinions but I see the two as potentially serving different target platforms as well as different target users.
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3481702959)
> > Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
>
> cc [@GBKS](https://github.com/GBKS) [@johnny9](https://github.com/johnny9)
My vote would be to continue to add features to the Qt Widgets gui. There might be differing opinions but I see the two as potentially serving different target platforms as well as different target users.
π¬ waketraindev commented on pull request "qt: add console commands for clearing output and history":
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3481744961)
Added release notes file.
(https://github.com/bitcoin-core/gui/pull/882#issuecomment-3481744961)
Added release notes file.
π€ hebasto reviewed a pull request: "Increase tooltip wrap threshold from 80 to 100 characters"
(https://github.com/bitcoin-core/gui/pull/905#pullrequestreview-3412212034)
This code was introduced in https://github.com/bitcoin/bitcoin/pull/1090. During that discussion, there was a [concern](https://github.com/bitcoin/bitcoin/pull/1090#issuecomment-5452893) that "tooltip windows [were] too wide sometimes".
This isnβt about fitting on the screen, but about improving readability.
cc @laanwj @GBKS
(https://github.com/bitcoin-core/gui/pull/905#pullrequestreview-3412212034)
This code was introduced in https://github.com/bitcoin/bitcoin/pull/1090. During that discussion, there was a [concern](https://github.com/bitcoin/bitcoin/pull/1090#issuecomment-5452893) that "tooltip windows [were] too wide sometimes".
This isnβt about fitting on the screen, but about improving readability.
cc @laanwj @GBKS
π¬ hulxv commented on pull request "feat: shell completions for bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481770602)
> Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.
Actually, bash is the only is implemented in #33385 but zsh isn't implemented in #33402
Also in #33385, he covers only `rpc`, `node`, and `tx`, but mine covers all subcommands
(https://github.com/bitcoin/bitcoin/pull/33747#issuecomment-3481770602)
> Thanks. However aside from the fish addition, this duplicates works already being done in #33385 and #33402.
Actually, bash is the only is implemented in #33385 but zsh isn't implemented in #33402
Also in #33385, he covers only `rpc`, `node`, and `tx`, but mine covers all subcommands