Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1133825558)
Can also add my test to check that no false warning will be issued anymore? Integer overflow isn't the only issue you are fixing here.
πŸ’¬ desirepl commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1466048126)
I see only translation changes 2 hours ago.
I'm not so familiar with git, so checked code updates here at main and other branches. Only your updates to translations 2 hours ago. Seems doing something wrong or don't understand, ΠΊΠΎΡ€ΠΎΡ‡Π΅.
πŸ’¬ MarcoFalke commented on pull request "wallet: ensure the wallet is unlocked when needed for rescanning":
(https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1133856319)
> However, for me this no longer fails without the fix.

For me, nothing fails. Unless I specifically add a manual sleep on the code before the fix:

```diff
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index eed40f9462..d5b5af0d4c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1851,6 +1851,8 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
WalletLogPrintf("Rescan started from block %s... (%s)\n", start_block.ToS
...
πŸ’¬ MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1133858582)
```suggestion
self.log.info("Test that no warning appears when assumed chain size fits on disk, but the prune target would not")
```

nit (if you retouch)
πŸ’¬ theStack commented on issue "RFC: Replacing Boost Process":
(https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049)
FWIW, I started working on replacing boost::process with cpp-subprocess on the following branch: https://github.com/theStack/bitcoin/tree/nuke_boost_process
Currently the unit tests (system_tests/run_command) and functional tests (rpc_signer.py, wallet_signer.py) pass on both Linux and OpenBSD, but compiling for Windows via g++-mingw-w64-x86-64-posix fails due to missing headers (`Windows.h`). The changes are not very invasive, most work was probably to change from passing a single string to a
...
πŸ€” MarcoFalke requested changes to a pull request: "test: fix race condition in encrypted wallet rescan tests"
(https://github.com/bitcoin/bitcoin/pull/27199)
Calling the cli constructor does not dispatch the rpc.

Otherwise this looks really nice.

I've tested this with https://github.com/bitcoin/bitcoin/pull/26347/commits/6a5b348f2e526f048d0b448b01f6c4ab608569af#r1133856319
πŸ’¬ MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1133892255)
```suggestion
minernode.cli("-rpcwallet=encrypted_wallet").walletlock()
```
πŸ’¬ MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1133892752)
```suggestion
self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletlock()
```
⚠️ red0bear opened an issue: "Adding interprocess onion access instead local ip"
(https://github.com/bitcoin/bitcoin/issues/27252)
### Please describe the feature you'd like to see added.

- onion uses 127.0.0.1:9050 and i would like to set it to use unix:path_to_variable used by tor

### Is your feature related to a problem, if so please describe it.

Avoid people try access local ips or 127.0.0.1 with miss configured torcc.

### Describe the solution you'd like

_No response_

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_
πŸ’¬ hebasto commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1466131399)
> I see only translation changes 2 hours ago. I'm not so familiar with git, so checked code updates here at main and other branches. Only your updates to translations 2 hours ago. Seems doing something wrong or don't understand, ΠΊΠΎΡ€ΠΎΡ‡Π΅.

I'm talking about changes since v24.0.1 release which was tested by you. For example, https://github.com/bitcoin/bitcoin/pull/27150.
πŸ‘ MarcoFalke approved a pull request: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)
ACK 05eeba2c5fb312e0e6a730b01eb7d1b422d75dbb πŸ₯

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 05eeba2c5fb312e0e6a730b01e
...
πŸ’¬ desirepl commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1466158043)
> I'm talking about changes since v24.0.1 release which was tested by you. For example, #27150.

Omg. I've cloned not the last code. I thought after "git tag" that 24.0.1 is the latest, not the rc's.
Ok, I'll try later when it's be possible. Now it isn't.
πŸ‘ stickies-v approved a pull request: "[24.x] qt: 24.1rc1 translations update"
(https://github.com/bitcoin/bitcoin/pull/27251)
I ran `bitcoin-maintainer-tools/update-translations.py` and verified that my diff matches with a2f8a839d9b071f9c90ed5e3db2d30f90993992d when discarding `bitcoin_nl.ts`.

I'm not familiar with the translations process and I couldn't verify that `bitcoin_nl.ts` is damaged, but if this constitutes sufficient review then ACK a2f8a839d9b071f9c90ed5e3db2d30f90993992d
πŸ’¬ willcl-ark commented on issue "Confusing log order regarding DNS seeds":
(https://github.com/bitcoin/bitcoin/issues/18332#issuecomment-1466253933)
I think this can be close now that the log messages are no longer returned in a confusing way since #16939:

```log
2023-03-13T14:30:03Z 0 block-relay-only anchors will be tried for connections.
2023-03-13T14:30:03Z init message: Starting network threads…
2023-03-13T14:30:03Z dnsseed thread start
2023-03-13T14:30:03Z init message: Done loading
2023-03-13T14:30:03Z opencon thread start
2023-03-13T14:30:03Z Loading addresses from DNS seed seed.bitcoin.sprovoost.nl.
2023-03-13T14:30:03Z ne
...
πŸš€ fanquake merged a pull request: "[24.x] qt: 24.1rc1 translations update"
(https://github.com/bitcoin/bitcoin/pull/27251)
βœ… MarcoFalke closed an issue: "Confusing log order regarding DNS seeds"
(https://github.com/bitcoin/bitcoin/issues/18332)
πŸ’¬ MarcoFalke commented on issue "Confusing log order regarding DNS seeds":
(https://github.com/bitcoin/bitcoin/issues/18332#issuecomment-1466270405)
Closing as per the two previous comments that this is fixed.
πŸš€ fanquake merged a pull request: "[24.x] Bump version to v24.1rc1"
(https://github.com/bitcoin/bitcoin/pull/27247)
πŸ‘ fanquake approved a pull request: "test: Default timeout factor to 4 under --valgrind"
(https://github.com/bitcoin/bitcoin/pull/27221)
ACK fa27cf4cc7c24aa00a66dffabab849d4b3cb1c41 - I still see at least one actual issue when running the functional tests under `--valgrind` (outside the CI system), but will follow up separately with that. Increasing the timeout here seems fine.
πŸš€ fanquake merged a pull request: "test: Default timeout factor to 4 under --valgrind"
(https://github.com/bitcoin/bitcoin/pull/27221)