β
MarcoFalke closed a pull request: "[WIP] DRAFT NOMERGE Tidy up RPCTxSerializationFlags"
(https://github.com/bitcoin/bitcoin/pull/23599)
(https://github.com/bitcoin/bitcoin/pull/23599)
π MarcoFalke approved a pull request: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)
lgtm
(https://github.com/bitcoin/bitcoin/pull/27235)
lgtm
π¬ MarcoFalke commented on pull request "Avoid integer overflow in CheckDiskSpace":
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1133824606)
```suggestion
self.log.info("Test that a manually pruned node does not run into integer overflow on first start")
```
nit: Not sure if more context is helpful. Otherwise, I suspect this test will be removed in the future due to being redundant.
(https://github.com/bitcoin/bitcoin/pull/27235#discussion_r1133824606)
```suggestion
self.log.info("Test that a manually pruned node does not run into integer overflow on first start")
```
nit: Not sure if more context is helpful. Otherwise, I suspect this test will be removed in the future due to being redundant.
π¬ 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.
(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, ΠΊΠΎΡΠΎΡΠ΅.
(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
...
(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)
(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
...
(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
(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()
```
(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()
```
(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_
(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.
(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
...
(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.
(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
(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
...
(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)
(https://github.com/bitcoin/bitcoin/pull/27251)
β
MarcoFalke closed an issue: "Confusing log order regarding DNS seeds"
(https://github.com/bitcoin/bitcoin/issues/18332)
(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.
(https://github.com/bitcoin/bitcoin/issues/18332#issuecomment-1466270405)
Closing as per the two previous comments that this is fixed.