💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193133568)
Shouldn't this change be moved to the first commit?
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193133568)
Shouldn't this change be moved to the first commit?
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193134480)
We also don't set other params, like `adjusted_time_callback` in the dummy opts, since they are not required to read the args into the struct. The commit where this is changed, adds it because the `BlockManager::Options` requires a reference to the notifications starting at that commit.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193134480)
We also don't set other params, like `adjusted_time_callback` in the dummy opts, since they are not required to read the args into the struct. The commit where this is changed, adds it because the `BlockManager::Options` requires a reference to the notifications starting at that commit.
💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193141876)
FWIW, gcc emits a warning here.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193141876)
FWIW, gcc emits a warning here.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193146924)
Thank you for following up, will patch.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193146924)
Thank you for following up, will patch.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546903132)
Re https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1425535676
> In commit [7355679](https://github.com/bitcoin/bitcoin/commit/73556797ab9c44038994954b945db996642656d2):
>
> ```diff
> --- a/src/validation.cpp
> +++ b/src/validation.cpp
> @@ -4659,7 +4659,7 @@ void Chainstate::LoadExternalBlockFile(
> }
> }
>
> - NotifyHeaderTip(*this);
> + ::NotifyHeaderTip(*this);
>
> if (!blo
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546903132)
Re https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1425535676
> In commit [7355679](https://github.com/bitcoin/bitcoin/commit/73556797ab9c44038994954b945db996642656d2):
>
> ```diff
> --- a/src/validation.cpp
> +++ b/src/validation.cpp
> @@ -4659,7 +4659,7 @@ void Chainstate::LoadExternalBlockFile(
> }
> }
>
> - NotifyHeaderTip(*this);
> + ::NotifyHeaderTip(*this);
>
> if (!blo
...
📝 theStack opened a pull request: "test: add unit test coverage for Python ECDSA implementation"
(https://github.com/bitcoin/bitcoin/pull/27653)
This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point.
To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose
...
(https://github.com/bitcoin/bitcoin/pull/27653)
This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point.
To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose
...
💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193161217)
Should these methods be non-`const`?
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193161217)
Should these methods be non-`const`?
💬 hebasto commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193161280)
nit: EOL?
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193161280)
nit: EOL?
💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1546917869)
I've fixed a bug which resulted in the conflicted tx not being marked as inactive if the conflicting tx was not in `mapTxSpends`.
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1546917869)
I've fixed a bug which resulted in the conflicted tx not being marked as inactive if the conflicting tx was not in `mapTxSpends`.
💬 ishaanam commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1193162426)
There are some tests for that here: https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_resendwallettransactions.py. Or did you mean that a test for that should be added for this PR specifically?
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1193162426)
There are some tests for that here: https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_resendwallettransactions.py. Or did you mean that a test for that should be added for this PR specifically?
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193165783)
Yes, as far as I am aware, `const` virtual methods are bad practice. See: https://github.com/bitcoin/bitcoin/pull/25337#discussion_r916367659.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193165783)
Yes, as far as I am aware, `const` virtual methods are bad practice. See: https://github.com/bitcoin/bitcoin/pull/25337#discussion_r916367659.
⚠️ bleeee opened an issue: "v0.6.5 on iPad mini 6 Security Center button incorrent"
(https://github.com/bitcoin/bitcoin/issues/27654)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
v0.6.5 on iPad mini 6 Security Center button incorrent
2FA button is actually app password
app password button is wall set pass phrase
all wallet encryption button is biometrics
### Expected behaviour
corrention buttons
### Steps to reproduce
press any button in secrity center
### Relevant log output
_No response_
### How did you obtain Bitcoin Core
Compiled from source
###
...
(https://github.com/bitcoin/bitcoin/issues/27654)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
v0.6.5 on iPad mini 6 Security Center button incorrent
2FA button is actually app password
app password button is wall set pass phrase
all wallet encryption button is biometrics
### Expected behaviour
corrention buttons
### Steps to reproduce
press any button in secrity center
### Relevant log output
_No response_
### How did you obtain Bitcoin Core
Compiled from source
###
...
✅ fanquake closed an issue: "v0.6.5 on iPad mini 6 Security Center button incorrent"
(https://github.com/bitcoin/bitcoin/issues/27654)
(https://github.com/bitcoin/bitcoin/issues/27654)
⚠️ kallerosenbaum opened an issue: "Can't start bitcoin-qt by double-click on linux"
(https://github.com/bitcoin/bitcoin/issues/27655)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When I double-click the bitcoin-qt file executable file in the graphical file manager, Bitcoin Core doesn't start and this message pops up:

I think this issue is the same as described at https://fostips.com/double-click-run-elf-ubuntu/
The Bitcoin Core build
...
(https://github.com/bitcoin/bitcoin/issues/27655)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When I double-click the bitcoin-qt file executable file in the graphical file manager, Bitcoin Core doesn't start and this message pops up:

I think this issue is the same as described at https://fostips.com/double-click-run-elf-ubuntu/
The Bitcoin Core build
...
📝 hebasto opened a pull request: "build, doc: Adjust comment after PR27254"
(https://github.com/bitcoin/bitcoin/pull/27656)
This is a follow up for https://github.com/bitcoin/bitcoin/pull/27254.
(https://github.com/bitcoin/bitcoin/pull/27656)
This is a follow up for https://github.com/bitcoin/bitcoin/pull/27254.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546926451)
Thank you for the review @hebasto,
Updated 63e915af5a437d36abdd79a1b90fed2395086589 -> 97b0e28d6f5e86e8c7ee0775d22cd923dabca09d ([chainstateRmKernelUiInterface_1](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_1) -> [chainstateRmKernelUiInterface_2](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_1..chainstateRmKernelUiInterface_2)).
* Add
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546926451)
Thank you for the review @hebasto,
Updated 63e915af5a437d36abdd79a1b90fed2395086589 -> 97b0e28d6f5e86e8c7ee0775d22cd923dabca09d ([chainstateRmKernelUiInterface_1](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_1) -> [chainstateRmKernelUiInterface_2](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_1..chainstateRmKernelUiInterface_2)).
* Add
...
💬 kallerosenbaum commented on issue "Can't start bitcoin-qt by double-click on linux":
(https://github.com/bitcoin/bitcoin/issues/27655#issuecomment-1546927537)
I also tested Bitcoin Core 24.0.1 on Ubuntu 22.04.2 LTS, and I could double click bitcoin-qt to start it there.
(https://github.com/bitcoin/bitcoin/issues/27655#issuecomment-1546927537)
I also tested Bitcoin Core 24.0.1 on Ubuntu 22.04.2 LTS, and I could double click bitcoin-qt to start it there.
💬 hebasto commented on issue "Can't start bitcoin-qt by double-click on linux":
(https://github.com/bitcoin/bitcoin/issues/27655#issuecomment-1546930882)
It seems like a Debian-specific problem, as I have no such an issue on Ubuntu (22.04, 23.04) with the downloaded binaries (v24.01, v25.0rc2).
Going to investigate further.
(https://github.com/bitcoin/bitcoin/issues/27655#issuecomment-1546930882)
It seems like a Debian-specific problem, as I have no such an issue on Ubuntu (22.04, 23.04) with the downloaded binaries (v24.01, v25.0rc2).
Going to investigate further.
👍 TheCharlatan approved a pull request: "build, doc: Adjust comment after PR27254"
(https://github.com/bitcoin/bitcoin/pull/27656#pullrequestreview-1425572673)
ACK 3ece0ebf627ee569a9680bf2663697db7a6ae309
(https://github.com/bitcoin/bitcoin/pull/27656#pullrequestreview-1425572673)
ACK 3ece0ebf627ee569a9680bf2663697db7a6ae309
👍 TheCharlatan approved a pull request: "Introduce `MockableDatabase` for wallet unit tests"
(https://github.com/bitcoin/bitcoin/pull/26715#pullrequestreview-1425572973)
ACK 33e2b82a4fc990253ff77655f437c7aed336bc55
(https://github.com/bitcoin/bitcoin/pull/26715#pullrequestreview-1425572973)
ACK 33e2b82a4fc990253ff77655f437c7aed336bc55