Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

electrum: make sure chain updates connect on rescan #1315

Merged

Conversation

darosior
Copy link
Member

The from_chain_tip command takes a CheckPoint of our current chain tip for purposes of providing a chain update which connects to it, including in case of a reorg.

This is not a point from which to rescan the chain.

The current code made it so in case a rescan was performed while there was too much progress on the backend (>8 blocks), or when there was a deep enough reorg, the chain update could not unambiguously connect to our local chain. Here is an instance (logs were adapted to debug this issue):

  2024-09-10T15:19:48.258728Z DEBUG liana::bitcoin::electrum:196: Full local chain: LocalChain { tip: CheckPoint(CPInner { block: BlockId { height: 618, hash: 0x6c6f30d76e1ce1bee2157d271300b93e968c501685c20e8466635310a4622efe }, prev: Some(CPInner { block: BlockId { height: 617, hash: 0x3ed1127ea206b672f47c6dfc984c63c229264f618015865057de9a75e7a6f8b0 }, prev: Some(CPInner { block: BlockId { height: 616, hash: 0x6ffe4d528
b5689802c46c86a1bd0acc9ecb35abf0bf73d2ccce72935b333c29e }, prev: Some(CPInner { block: BlockId { height: 615, hash: 0x433f6d69da936c29fdf02d43aeca9e0cfce0ce877f628743ada38e566dff1d73 }, prev: Some(CPInner { block: BlockId { height: 614, hash: 0x12a64d6416e196618154c34ad58f5c7342b59a752dbc4686af668e987f38dc28 }, prev: Some(CPInner { block: BlockId { height: 613, hash: 0x539942c4bb38f94e9ae35f0d120c8e250edd7a56930e130fe079 59d179dd2c0a }, prev: Some(CPInner { block: BlockId { height: 612, hash: 0x12ab45ad9c2dc246e450f98831cf2e6c5b3c9c5a0ab4af46c8a8cd915680d728 }, prev: Some(CPInner { block: BlockId { height: 611, hash: 0x21c890e0885f4672e22dbb6d17b110863bdd6b0464e0f5e731072d63f45a12d7 }, prev: Some(CPInner { block: BlockId { height: 515, hash: 0x021e3c76fbe92630214b1b23b85d44a9b443c6b1d01b336b7d1994a18944ebb1 }, prev: Some(CPInner { block:
 BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) }) }) }

  2024-09-10T15:19:48.259058Z DEBUG liana::bitcoin::electrum:197: Full chain update: CheckPoint(CPInner { block: BlockId { height: 633, hash: 0x41ce59abb80de83d165501a1f9ba7acc98ed0ce4af7fde512fd2491905e777bd }, prev: Some(CPInner { block: BlockId { height: 632, hash: 0x4e812e2a98150c5da85efa44d792feed64e76ffbd0bf31ac5925a1b798bad959 }, prev: Some(CPInner { block: BlockId { height: 631, hash: 0x1379b6c609ce9adb0e2123dd36
629e71a5011dc0e9f6aa3935f2f6c5c9ed28b2 }, prev: Some(CPInner { block: BlockId { height: 630, hash: 0x4bb4f27e5cefd3ef2d9707af6ed933c6bb049a48081250d365fdb1bd3a10da4d }, prev: Some(CPInner { block: BlockId { height: 629, hash: 0x64bf474cde129b6b9a45c7f697d57c9514ef33de02cda3df1f65543f0b111366 }, prev: Some(CPInner { block: BlockId { height: 628, hash: 0x60c205d56d5a244da352867c7db48a696fe9ecfe04d0fd4f105c7663afdf5717 }, p
rev: Some(CPInner { block: BlockId { height: 627, hash: 0x70b8d4091481295e39582d82f5c72ca5cb62505861a6d804afc1b1a0caa25c2b }, prev: Some(CPInner { block: BlockId { height: 626, hash: 0x15ad17258b32d87b8719e6ee911e4b54e8095a3429d05617d391f262a4e8b8c1 }, prev: Some(CPInner { block: BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) })

I think this is the root cause of the issue we are seeing in the rescan_edge_cases functional test when ran in CI, and also of the crash Pyth reported.

Fixes #1280.
Fixes #1270.

The `from_chain_tip` command takes a `CheckPoint` of our current chain
tip for purposes of providing a chain update which connects to it,
including in case of a reorg.

This is not a point from which to rescan the chain.

The current code made it so in case a rescan was performed while there
was too much progress on the backend (>8 blocks), or when there was a
deep enough reorg, the chain update could not unambiguously connect to
our local chain. Here is an instance (logs were adapted to debug this
issue):
  2024-09-10T15:19:48.258728Z DEBUG liana::bitcoin::electrum:196: Full local chain: LocalChain { tip: CheckPoint(CPInner { block: BlockId { height: 618, hash: 0x6c6f30d76e1ce1bee2157d271300b93e968c501685c20e8466635310a4622efe }, prev: Some(CPInner { block: BlockId { height: 617, hash: 0x3ed1127ea206b672f47c6dfc984c63c229264f618015865057de9a75e7a6f8b0 }, prev: Some(CPInner { block: BlockId { height: 616, hash: 0x6ffe4d528
b5689802c46c86a1bd0acc9ecb35abf0bf73d2ccce72935b333c29e }, prev: Some(CPInner { block: BlockId { height: 615, hash: 0x433f6d69da936c29fdf02d43aeca9e0cfce0ce877f628743ada38e566dff1d73 }, prev: Some(CPInner { block: BlockId { height: 614, hash: 0x12a64d6416e196618154c34ad58f5c7342b59a752dbc4686af668e987f38dc28 }, prev: Some(CPInner { block: BlockId { height: 613, hash: 0x539942c4bb38f94e9ae35f0d120c8e250edd7a56930e130fe079
59d179dd2c0a }, prev: Some(CPInner { block: BlockId { height: 612, hash: 0x12ab45ad9c2dc246e450f98831cf2e6c5b3c9c5a0ab4af46c8a8cd915680d728 }, prev: Some(CPInner { block: BlockId { height: 611, hash: 0x21c890e0885f4672e22dbb6d17b110863bdd6b0464e0f5e731072d63f45a12d7 }, prev: Some(CPInner { block: BlockId { height: 515, hash: 0x021e3c76fbe92630214b1b23b85d44a9b443c6b1d01b336b7d1994a18944ebb1 }, prev: Some(CPInner { block:
 BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) }) }) }

  2024-09-10T15:19:48.259058Z DEBUG liana::bitcoin::electrum:197: Full chain update: CheckPoint(CPInner { block: BlockId { height: 633, hash: 0x41ce59abb80de83d165501a1f9ba7acc98ed0ce4af7fde512fd2491905e777bd }, prev: Some(CPInner { block: BlockId { height: 632, hash: 0x4e812e2a98150c5da85efa44d792feed64e76ffbd0bf31ac5925a1b798bad959 }, prev: Some(CPInner { block: BlockId { height: 631, hash: 0x1379b6c609ce9adb0e2123dd36
629e71a5011dc0e9f6aa3935f2f6c5c9ed28b2 }, prev: Some(CPInner { block: BlockId { height: 630, hash: 0x4bb4f27e5cefd3ef2d9707af6ed933c6bb049a48081250d365fdb1bd3a10da4d }, prev: Some(CPInner { block: BlockId { height: 629, hash: 0x64bf474cde129b6b9a45c7f697d57c9514ef33de02cda3df1f65543f0b111366 }, prev: Some(CPInner { block: BlockId { height: 628, hash: 0x60c205d56d5a244da352867c7db48a696fe9ecfe04d0fd4f105c7663afdf5717 }, p
rev: Some(CPInner { block: BlockId { height: 627, hash: 0x70b8d4091481295e39582d82f5c72ca5cb62505861a6d804afc1b1a0caa25c2b }, prev: Some(CPInner { block: BlockId { height: 626, hash: 0x15ad17258b32d87b8719e6ee911e4b54e8095a3429d05617d391f262a4e8b8c1 }, prev: Some(CPInner { block: BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) })
Copy link
Collaborator

@jp1ac4 jp1ac4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK d3082a6.

@darosior
Copy link
Member Author

After tinkering on the CI's terminal for a while i finally noticed there was actually a mistake in the rescan_edge_cases functional test. Just pushed a commit to this effect.

I also noticed sometimes with an Electrum backend the test could fail with:

Click to see functional test error
    def test_rescan_edge_cases(lianad, bitcoind):
        """Test some specific cases that could arise when rescanning the chain."""
        initial_tip = bitcoind.rpc.getblockheader(bitcoind.rpc.getbestblockhash())
    
        # Some helpers
        list_coins = lambda: lianad.rpc.listcoins()["coins"]
        sorted_coins = lambda: sorted(list_coins(), key=lambda c: c["outpoint"])
        def wait_synced():
            l = lianad.rpc.getinfo()["block_height"]
            b = bitcoind.rpc.getblockcount()
            print(f"In wait_synced l {l} b {b}")
            return l == b
    
        def reorg_shift(height, txs):
            """Remine the chain from given height, shifting the txs by one block."""
            delta = bitcoind.rpc.getblockcount() - height + 1
            print(f"height: {height}, delta {delta}")
            assert delta > 2
            h = bitcoind.rpc.getblockhash(initial_tip["height"])
            bitcoind.rpc.invalidateblock(h)
            b = bitcoind.rpc.getblockcount()
            print(f"block count after invalidation {b}")
            bitcoind.generate_block(1)
            b = bitcoind.rpc.getblockcount()
            print(f"block count after 1 block remine {b}")
            for tx in txs:
                bitcoind.rpc.sendrawtransaction(tx)
            bitcoind.generate_block(delta - 1, wait_for_mempool=len(txs))
            b = bitcoind.rpc.getblockcount()
            print(f"block count after {delta - 1} blocks remine {b}")
    
        # Create 3 coins and spend 2 of them. Keep the transactions in memory to
        # rebroadcast them on reorgs.
        txs = []
        for _ in range(3):
            addr = lianad.rpc.getnewaddress()["address"]
            amount = 0.356
            txid = bitcoind.rpc.sendtoaddress(addr, amount)
            txs.append(bitcoind.rpc.gettransaction(txid)["hex"])
        wait_for(lambda: len(list_coins()) == 3)
        txs.append(spend_coins(lianad, bitcoind, list_coins()[:2]))
        bitcoind.generate_block(1, wait_for_mempool=4)
        wait_synced()
    
        # Advance the blocktime by >2h in the future for the importdescriptors rescan
        added_time = 60 * 60 * 3
        bitcoind.rpc.setmocktime(initial_tip["time"] + added_time)
        bitcoind.generate_block(12)
    
        # Lose our state
        coins_before = sorted_coins()
        outpoints_before = set(c["outpoint"] for c in coins_before)
        bitcoind.generate_block(1)
        lianad.restart_fresh(bitcoind)
        if BITCOIN_BACKEND_TYPE is BitcoinBackendType.Bitcoind:
            assert len(list_coins()) == 0
    
        # We can be stopped while we are rescanning
        lianad.rpc.startrescan(initial_tip["time"])
        lianad.stop()
        lianad.start()
        wait_for(lambda: lianad.rpc.getinfo()["rescan_progress"] is None)
        wait_synced()
>       assert coins_before == sorted_coins()
E       AssertionError: assert [{'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': None, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': None}, {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j', 'amount': 35600000, 'block_height': None, 'derivation_index': 2, 'is_change': False, 'is_immature': False, 'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0', 'spend_info': None}, {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp', 'amount': 35600000, 'block_height': None, 'derivation_index': 1, 'is_change': False, 'is_immature': False, 'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1', 'spend_info': None}] == [{'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': 102, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}, {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j', 'amount': 35600000, 'block_height': 102, 'derivation_index': 2, 'is_change': False, 'is_immature': False, 'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0', 'spend_info': None}, {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp', 'amount': 35600000, 'block_height': 102, 'derivation_index': 1, 'is_change': False, 'is_immature': False, 'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}]
E         At index 0 diff: {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': None, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': None} != {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': 102, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}
E         Full diff:
E           [
E            {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30',
E             'amount': 35600000,
E         -   'block_height': 102,
E         ?                   ^^^
E         +   'block_height': None,
E         ?                   ^^^^
E             'derivation_index': 0,
E             'is_change': False,
E             'is_immature': False,
E             'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1',
E         +   'spend_info': None},
E         -   'spend_info': {'height': 102,
E         -                  'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}},
E            {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j',
E             'amount': 35600000,
E         -   'block_height': 102,
E         ?                   ^^^
E         +   'block_height': None,
E         ?                   ^^^^
E             'derivation_index': 2,
E             'is_change': False,
E             'is_immature': False,
E             'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0',
E             'spend_info': None},
E            {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp',
E             'amount': 35600000,
E         -   'block_height': 102,
E         ?                   ^^^
E         +   'block_height': None,
E         ?                   ^^^^
E             'derivation_index': 1,
E             'is_change': False,
E             'is_immature': False,
E             'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1',
E         +   'spend_info': None},
E         -   'spend_info': {'height': 102,
E         -                  'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}},
E           ]

tests/test_chain.py:259: AssertionError
------------------------------------------------- Captured stdout setup --------------------------------------------------
Running tests in /tmp/lianad-tests-eqmhg229
-------------------------------------------------- Captured stdout call --------------------------------------------------
In wait_synced l 101 b 102
In wait_synced l 115 b 115
------------------------------------------------ Captured stdout teardown ------------------------------------------------
Test failed, leaving directory '/tmp/lianad-tests-eqmhg229/test_rescan_edge_cases_1' intact
Leaving base dir '/tmp/lianad-tests-eqmhg229' as it still contains ['test_rescan_edge_cases_1']
================================================ short test summary info =================================================
FAILED tests/test_chain.py::test_rescan_edge_cases - AssertionError: assert [{'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': None, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': None}, {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j', 'amount': 35600000, 'block_height': None, 'derivation_index': 2, 'is_change': False, 'is_immature': False, 'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0', 'spend_info': None}, {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp', 'amount': 35600000, 'block_height': None, 'derivation_index': 1, 'is_change': False, 'is_immature': False, 'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1', 'spend_info': None}] == [{'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': 102, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}, {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j', 'amount': 35600000, 'block_height': 102, 'derivation_index': 2, 'is_change': False, 'is_immature': False, 'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0', 'spend_info': None}, {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp', 'amount': 35600000, 'block_height': 102, 'derivation_index': 1, 'is_change': False, 'is_immature': False, 'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}]
  At index 0 diff: {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': None, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': None} != {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30', 'amount': 35600000, 'block_height': 102, 'derivation_index': 0, 'is_change': False, 'is_immature': False, 'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1', 'spend_info': {'height': 102, 'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}}
  Full diff:
    [
     {'address': 'bcrt1qeyfupz7tf887t7qn3aj7k8lmcr6gqrxwzaynly37chvegmdt29zqjy0k30',
      'amount': 35600000,
  -   'block_height': 102,
  ?                   ^^^
  +   'block_height': None,
  ?                   ^^^^
      'derivation_index': 0,
      'is_change': False,
      'is_immature': False,
      'outpoint': 'b28bc57d789d8446899f35034e15bf85ff226132c81f0d6573d2cd017561aa03:1',
  +   'spend_info': None},
  -   'spend_info': {'height': 102,
  -                  'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}},
     {'address': 'bcrt1q26pfkpy99fwg8vy2652myg40zhgvkwj0d7htgp0u5zylxyl0vcjqmzkj6j',
      'amount': 35600000,
  -   'block_height': 102,
  ?                   ^^^
  +   'block_height': None,
  ?                   ^^^^
      'derivation_index': 2,
      'is_change': False,
      'is_immature': False,
      'outpoint': 'd80840d44fdece0f7a0ff5608db4e9b2d910bdc07d00d622d084c54ebb4365e7:0',
      'spend_info': None},
     {'address': 'bcrt1qthl00a2vklxx7pt3funjzghv5r4wfs5gh87ynker88qvdccs3enq66fgdp',
      'amount': 35600000,
  -   'block_height': 102,
  ?                   ^^^
  +   'block_height': None,
  ?                   ^^^^
      'derivation_index': 1,
      'is_change': False,
      'is_immature': False,
      'outpoint': 'eae306a9b0e2764b5c9b6ef5d37bdf0a69ddc66ba0e061eff5019994b7cb7d1c:1',
  +   'spend_info': None},
  -   'spend_info': {'height': 102,
  -                  'txid': 'bdc5342ba5a60fdc4e67e932b69d2a6fe2c0eb37f9a31d592f7696dcceff5a75'}},
    ]
=================================================== 1 failed in 7.78s ====================================================

This can be fixed with:

diff --git a/tests/test_chain.py b/tests/test_chain.py
index 30027bb..92753ca 100644
--- a/tests/test_chain.py
+++ b/tests/test_chain.py
@@ -227,6 +227,7 @@ def test_rescan_edge_cases(lianad, bitcoind):
     txs.append(spend_coins(lianad, bitcoind, list_coins()[:2]))
     bitcoind.generate_block(1, wait_for_mempool=4)
     wait_synced()
+    wait_for(lambda: len(lianad.rpc.listcoins(["spent"])["coins"]) == 2)
 
     # Advance the blocktime by >2h in the future for the importdescriptors rescan
     added_time = 60 * 60 * 3

But this should not be necessary since we wait for lianad to be synced just the line before! It means with an Electrum backend somehow lianad could be at the correct height and yet not have all updates about coins?

@darosior
Copy link
Member Author

Alright, this fixed the CI. I'll clean up the branch, which will trigger CI again as a sanity check. I think there is still potential for flakiness due to the issue detailed in my previous comment. Let's get back to this if we notice flakiness being reintroduced.

@darosior
Copy link
Member Author

Actually i'll keep the debug log trace for now. It seems pretty useful to have.

2 minutes should be more than enough, this more than halves the time to
  detect something's wrong!
It was using the passed height for all other calculation, except for the
invalidation itself where it was using the wider-scope-initial-tip! This
is probably a mistake due to moving it to its own, inner, function.
@darosior darosior force-pushed the 2409_electrum_fix_unconnected_rescans branch from af5ca6a to 3d7b3e4 Compare September 11, 2024 08:12
@darosior
Copy link
Member Author

Kept the lower CI timeout too so we don't have to wait 5 full minutes to understand a test is stalled.

Copy link
Collaborator

@jp1ac4 jp1ac4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3d7b3e4.

@darosior darosior merged commit 4dd0fd3 into wizardsardine:master Sep 11, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Electrum sync/rescan crash electrum: fix flaky reorg tests
3 participants