💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180843713)
Ah, I missed this - then it should be fine!
  (https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180843713)
Ah, I missed this - then it should be fine!
💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180834640)
I still think it would be sufficient to only set it for the tip - I don't think that the edge case of `invalidateblock` justifies doing another iteration over all blocks during each init (it takes 0.1 - 0.2s on mainnet for me on a reasonably fast laptop, which is not that much, but I would imagine it might be quite a bit longer with slower hardware / longer chains such as testnet3 - unless there is another reason to set it for all blocks that I am unaware of.
  (https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2180834640)
I still think it would be sufficient to only set it for the tip - I don't think that the edge case of `invalidateblock` justifies doing another iteration over all blocks during each init (it takes 0.1 - 0.2s on mainnet for me on a reasonably fast laptop, which is not that much, but I would imagine it might be quite a bit longer with slower hardware / longer chains such as testnet3 - unless there is another reason to set it for all blocks that I am unaware of.
👍 pinheadmz approved a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2980431502)
ACK ef2a013e31cf6fbded5735a998b4c992c176493d
Build on macos/arm64 after experiencing issues locally building a checked-out v29.0 GUI with only qt 6 installed. Can confirm these backports solve that problem and GUI runs fine. Did not test the other backports.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK ef2a013e31cf6fbded5735a998b4c992c176493d
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhli/0AC
...
  (https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2980431502)
ACK ef2a013e31cf6fbded5735a998b4c992c176493d
Build on macos/arm64 after experiencing issues locally building a checked-out v29.0 GUI with only qt 6 installed. Can confirm these backports solve that problem and GUI runs fine. Did not test the other backports.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK ef2a013e31cf6fbded5735a998b4c992c176493d
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhli/0AC
...
🤔 furszy reviewed a pull request: "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs"
(https://github.com/bitcoin/bitcoin/pull/32618#pullrequestreview-2980491758)
light code review ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055
  (https://github.com/bitcoin/bitcoin/pull/32618#pullrequestreview-2980491758)
light code review ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055
💬 furszy commented on issue "Assertion failed: TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state) (wallet/wallet.cpp: AddToWallet: 1094)":
(https://github.com/bitcoin/bitcoin/issues/32625#issuecomment-3029194004)
can be closed.
  (https://github.com/bitcoin/bitcoin/issues/32625#issuecomment-3029194004)
can be closed.
💬 achow101 commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180895640)
Will do if I need to retouch
  (https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180895640)
Will do if I need to retouch
💬 achow101 commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180895752)
Will do if I need to retouch
  (https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180895752)
Will do if I need to retouch
💬 achow101 commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180896090)
Will do if I need to retouch
  (https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2180896090)
Will do if I need to retouch
✅ achow101 closed an issue: "Assertion failed: TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state) (wallet/wallet.cpp: AddToWallet: 1094)"
(https://github.com/bitcoin/bitcoin/issues/32625)
  (https://github.com/bitcoin/bitcoin/issues/32625)
💬 l0rinc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3029221330)
> I would suggest investigating the number of "generated table ..." logs
The "generated table" messages from LevelDB are normal when you enable debug logs. If you want to see real progress, turn those logs off.
v29 includes a related LevelDB optimization which may help a bit: https://github.com/bitcoin/bitcoin/pull/30039
> Improvement by 10-20% is below my expectations. I'm looking for 1000%.
We’d all love a 10x speed-up. Feel free to implement it and share it with us.
----
But quickly sca
...
  (https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3029221330)
> I would suggest investigating the number of "generated table ..." logs
The "generated table" messages from LevelDB are normal when you enable debug logs. If you want to see real progress, turn those logs off.
v29 includes a related LevelDB optimization which may help a bit: https://github.com/bitcoin/bitcoin/pull/30039
> Improvement by 10-20% is below my expectations. I'm looking for 1000%.
We’d all love a 10x speed-up. Feel free to implement it and share it with us.
----
But quickly sca
...
🚀 achow101 merged a pull request: "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet"
(https://github.com/bitcoin/bitcoin/pull/31423)
  (https://github.com/bitcoin/bitcoin/pull/31423)
👍 instagibbs approved a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2980538397)
reACK 1632fc104be8f171f59a023800b2f9f20d0a3cff
Includes the benchmark and unit test for trimming singletons, and a text cleanup
`git range-diff master 2b2df98747fdb6380588991167ce2e8cb92f3bfb 1632fc104be8f171f59a023800b2f9f20d0a3cff`
  (https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2980538397)
reACK 1632fc104be8f171f59a023800b2f9f20d0a3cff
Includes the benchmark and unit test for trimming singletons, and a text cleanup
`git range-diff master 2b2df98747fdb6380588991167ce2e8cb92f3bfb 1632fc104be8f171f59a023800b2f9f20d0a3cff`
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180919200)
fwiw I commented out the IsOversized check and the test passes still as expected, with singletons
  (https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180919200)
fwiw I commented out the IsOversized check and the test passes still as expected, with singletons
💬 achow101 commented on pull request "test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()":
(https://github.com/bitcoin/bitcoin/pull/32823#issuecomment-3029236281)
ACK ec004cdb86e6471915e1033f390c76ee0428e415
  (https://github.com/bitcoin/bitcoin/pull/32823#issuecomment-3029236281)
ACK ec004cdb86e6471915e1033f390c76ee0428e415
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3029241989)
Concept ACK e36e0b1aebe437c090ff897d27bbec0a7c5a100a
**Concept**: Theory for intermittent failure in #32600 makes sense, better to wait for all indexes before shutting down.
**Testing**: Successfully tested with the suggested diff modifying `BaseIndex::Sync()` to prevent syncing.
**Approach**: Left some nits, but most important thing I found was a comment that was probably accidentally removed.
  (https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3029241989)
Concept ACK e36e0b1aebe437c090ff897d27bbec0a7c5a100a
**Concept**: Theory for intermittent failure in #32600 makes sense, better to wait for all indexes before shutting down.
**Testing**: Successfully tested with the suggested diff modifying `BaseIndex::Sync()` to prevent syncing.
**Approach**: Left some nits, but most important thing I found was a comment that was probably accidentally removed.
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180042660)
nit: It might be good to send in the same `startup_args` to `check_clean_start()` further down in the loop?
https://github.com/bitcoin/bitcoin/blob/e36e0b1aebe437c090ff897d27bbec0a7c5a100a/test/functional/feature_init.py#L192
(Side remark: It is unbalanced that we don't call `check_clean_start()` at the bottom of the perturbation-loop, but saves execution time and should be alright as we check for different error messages).
  (https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180042660)
nit: It might be good to send in the same `startup_args` to `check_clean_start()` further down in the loop?
https://github.com/bitcoin/bitcoin/blob/e36e0b1aebe437c090ff897d27bbec0a7c5a100a/test/functional/feature_init.py#L192
(Side remark: It is unbalanced that we don't call `check_clean_start()` at the bottom of the perturbation-loop, but saves execution time and should be alright as we check for different error messages).
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180023938)
nit: Could simplify:
```python
def check_clean_start(args=[]):
"""Ensure that node restarts successfully after various interrupts."""
node.start(args)
```
  (https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180023938)
nit: Could simplify:
```python
def check_clean_start(args=[]):
"""Ensure that node restarts successfully after various interrupts."""
node.start(args)
```
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180836207)
This comment for the perturbed files is lost, unlike the one for the deleted files above.
  (https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180836207)
This comment for the perturbed files is lost, unlike the one for the deleted files above.
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180033937)
nit: Could add quotes and make output as verbose as variables:
```suggestion
self.log.info(f"Expecting error fragment: {err_fragment!r}")
```
  (https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180033937)
nit: Could add quotes and make output as verbose as variables:
```suggestion
self.log.info(f"Expecting error fragment: {err_fragment!r}")
```
💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180861847)
nit: Could use `@dataclass` for slightly less stringly typed code, and `tuple` for immutability:
<details><summary>Expand for diff</summary>
```diff
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 6ee075f906..1913002962 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -9,6 +9,7 @@ import platform
import shutil
import signal
import subprocess
+from dataclasses import dataclass, field
 
from test_framework
...
  (https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180861847)
nit: Could use `@dataclass` for slightly less stringly typed code, and `tuple` for immutability:
<details><summary>Expand for diff</summary>
```diff
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 6ee075f906..1913002962 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -9,6 +9,7 @@ import platform
import shutil
import signal
import subprocess
+from dataclasses import dataclass, field
from test_framework
...
