Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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?
💬 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.
💬 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.
💬 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.
💬 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
...
📝 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
...
💬 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`?
💬 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?
💬 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`.
💬 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?
💬 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.
⚠️ 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

###
...
fanquake closed an issue: "v0.6.5 on iPad mini 6 Security Center button incorrent"
(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:

![Screenshot from 2023-05-13 10-39-57](https://github.com/bitcoin/bitcoin/assets/1530071/a0e956e3-7a13-4288-a7fc-96738667c63c)

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.
💬 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
...
💬 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.
💬 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.
👍 TheCharlatan approved a pull request: "build, doc: Adjust comment after PR27254"
(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