💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380001489)
I think I understood that, but that will be a behavior change, since currently after `Enabling... too recent...` we're still getting a log for `Enabling ... block height above...` while if we only check empty vs non-empty we'd only be logging `Disabled` -> `Enabled` (or vice versa) but not slightly more confusing cases like the one above. Given that the logging was added because even we got confused by this, I thought we should log every state change.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380001489)
I think I understood that, but that will be a behavior change, since currently after `Enabling... too recent...` we're still getting a log for `Enabling ... block height above...` while if we only check empty vs non-empty we'd only be logging `Disabled` -> `Enabled` (or vice versa) but not slightly more confusing cases like the one above. Given that the logging was added because even we got confused by this, I thought we should log every state change.
💬 andrewtoth commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380011100)
I don't understand why you think it's a behavior change. It has nothing to do with logging. Add this on line 2585: `const bool fScriptChecks = (script_check_reason != nullptr);`
Remove changes on lines 2594, 2653.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380011100)
I don't understand why you think it's a behavior change. It has nothing to do with logging. Add this on line 2585: `const bool fScriptChecks = (script_check_reason != nullptr);`
Remove changes on lines 2594, 2653.
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2380012998)
Done
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2380012998)
Done
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2380013207)
Fixed
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2380013207)
Fixed
💬 l0rinc commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2380061485)
While the change does seem to fix my personal experience of unexpected script validations, I don't yet understand why reindex does this in the first place - can we fix that instead of adding extra conditions to the script validation checks? Seems a bit hacky and only solves part of the problem, doesn't it?
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2380061485)
While the change does seem to fix my personal experience of unexpected script validations, I don't yet understand why reindex does this in the first place - can we fix that instead of adding extra conditions to the script validation checks? Seems a bit hacky and only solves part of the problem, doesn't it?
💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380068845)
> I don't understand why you think it's a behavior change
Compared to the latest state of the code it's a change (not compared `master`).
If we only log when the nullnes changes that will be a different behavior than when the reason changes.
Or am I misunderstanding the comments?
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380068845)
> I don't understand why you think it's a behavior change
Compared to the latest state of the code it's a change (not compared `master`).
If we only log when the nullnes changes that will be a different behavior than when the reason changes.
Or am I misunderstanding the comments?
💬 l0rinc commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380071174)
Right, so what's the advantage of `getblockcount() == 1` compared to "non-zero"?
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380071174)
Right, so what's the advantage of `getblockcount() == 1` compared to "non-zero"?
🚀 glozow merged a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230)
(https://github.com/bitcoin/bitcoin/pull/33230)
💬 glozow commented on pull request "refactor: unify container presence checks (without PR conflicts)":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3335715733)
Apologies, this needs rebase for #33230. I decided to merge it first since it's a functional change and has already gone through a reACK cycle. This one is also very straightforward to rebase.
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3335715733)
Apologies, this needs rebase for #33230. I decided to merge it first since it's a functional change and has already gone through a reACK cycle. This one is also very straightforward to rebase.
💬 achow101 commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3335809587)
ACK a5e4fec4949f61ebbd7d6696f39da26df04515f9
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3335809587)
ACK a5e4fec4949f61ebbd7d6696f39da26df04515f9
💬 hodlinator commented on pull request "log: print every script verification state change":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380262207)
Makes the int -> bool coercion explicit to the reader, see https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362266224
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2380262207)
Makes the int -> bool coercion explicit to the reader, see https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2362266224
🚀 achow101 merged a pull request: "[28.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/33415)
(https://github.com/bitcoin/bitcoin/pull/33415)
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380282194)
Testing a proper Qt interface for better formatting now.
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380282194)
Testing a proper Qt interface for better formatting now.
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380297519)
move ChallengeToStdString to src/util/strencodings.cpp for later availability in main window.
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380297519)
move ChallengeToStdString to src/util/strencodings.cpp for later availability in main window.
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380298450)
removed std::string RPCConsole::challengeToString(const std::vector<uint8_t>& v)
(https://github.com/bitcoin-core/gui/pull/896#discussion_r2380298450)
removed std::string RPCConsole::challengeToString(const std::vector<uint8_t>& v)
💬 D33r-Gee commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-3335902873)
With [82a3969](https://github.com/bitcoin-core/gui/commit/82a3969f6570d370943fb3a7be86cd3ffdacdd4d) it updates the interface methods to match https://github.com/bitcoin/bitcoin/pull/33117
It also adds a load snapshot button during the intro and seamless loading if a proper path has been selected (from @luke-jr suggestion)
Also adds the use of threads to make sure there's no hanging or UI blocking when the load snapshot button is used in the options dialog (hopefully addressing @Sjors experie
...
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-3335902873)
With [82a3969](https://github.com/bitcoin-core/gui/commit/82a3969f6570d370943fb3a7be86cd3ffdacdd4d) it updates the interface methods to match https://github.com/bitcoin/bitcoin/pull/33117
It also adds a load snapshot button during the intro and seamless loading if a proper path has been selected (from @luke-jr suggestion)
Also adds the use of threads to make sure there's no hanging or UI blocking when the load snapshot button is used in the options dialog (hopefully addressing @Sjors experie
...
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335904661)
added "trivial challenge" support per signet implementation.
<img width="1062" height="613" alt="Screenshot 2025-09-25 at 4 47 29 PM" src="https://github.com/user-attachments/assets/d09860cf-2578-4b55-95f2-7a8f948909d6" />
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335904661)
added "trivial challenge" support per signet implementation.
<img width="1062" height="613" alt="Screenshot 2025-09-25 at 4 47 29 PM" src="https://github.com/user-attachments/assets/d09860cf-2578-4b55-95f2-7a8f948909d6" />
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335910910)
handle long challenges with proper elision and text flow.
<img width="1062" height="597" alt="Screenshot 2025-09-25 at 4 24 29 PM" src="https://github.com/user-attachments/assets/0f1d7b97-77c9-48f6-ae43-7b72e7fd8084" />
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335910910)
handle long challenges with proper elision and text flow.
<img width="1062" height="597" alt="Screenshot 2025-09-25 at 4 24 29 PM" src="https://github.com/user-attachments/assets/0f1d7b97-77c9-48f6-ae43-7b72e7fd8084" />
💬 RandyMcMillan commented on pull request "rpcconsole: display signet challenge":
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335914267)
common length challenges elide and flow correctly.
<img width="1078" height="673" alt="Screenshot 2025-09-25 at 4 25 50 PM" src="https://github.com/user-attachments/assets/f76d9839-ae13-4f70-aeea-0a268ecfad19" />
(https://github.com/bitcoin-core/gui/pull/896#issuecomment-3335914267)
common length challenges elide and flow correctly.
<img width="1078" height="673" alt="Screenshot 2025-09-25 at 4 25 50 PM" src="https://github.com/user-attachments/assets/f76d9839-ae13-4f70-aeea-0a268ecfad19" />
💬 achow101 commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3335956226)
ACK 453b0fa286e5dce0af682b7b73684dd6415a50de
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3335956226)
ACK 453b0fa286e5dce0af682b7b73684dd6415a50de