JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
731 stars 179 forks source link

Wallet recovery fails - "cannot advance index without force=True" #1188

Open mandelbit opened 2 years ago

mandelbit commented 2 years ago

When using the wallet-tool.py recover method, I'm encountering the error in the title. Error trace:

Traceback (most recent call last):
  File "wallet-tool.py", line 6, in <module>
    jmprint(wallet_tool_main("wallets"), "success")
  File "/home/joinmarket/joinmarket-clientserver/jmclient/jmclient/wallet_utils.py", line 1516, in wallet_tool_main
    if wallet_service.sync_wallet(fast = not options.recoversync):
  File "/home/joinmarket/joinmarket-clientserver/jmclient/jmclient/wallet_service.py", line 453, in sync_wallet
    self.sync_addresses()
  File "/home/joinmarket/joinmarket-clientserver/jmclient/jmclient/wallet_service.py", line 741, in sync_addresses
    self.sync_burner_outputs(burner_txes)
  File "/home/joinmarket/joinmarket-clientserver/jmclient/jmclient/wallet_service.py", line 663, in sync_burner_outputs
    self.wallet.set_next_index(mixdepth, address_type, highest_used_index + 1)
  File "/home/joinmarket/joinmarket-clientserver/jmclient/jmclient/wallet.py", line 2207, in set_next_index
    raise Exception("cannot advance index without force=True")
Exception: cannot advance index without force=True

Haven't noticing this error before when importing wallets from seeds. Is this a simple bug where force=True should have been set or is that unsafe with wallet import/recovery? Haven't had a chance to dive deeper into the code yet but can do so and try a PR if an appropriate fix is that simple.

Otherwise, please can someone more familiar with the codebase offer support on likely causes/resolutions. Have been unable to find reports of related issues here/telegram.

AdamISZ commented 2 years ago

@mandelbit hi thanks for the report. Can you give the commit you're running off?

mandelbit commented 2 years ago

Thanks for getting back to me so quickly - running off commit 868e188206b9645437e41801abca256961739126

AdamISZ commented 2 years ago

No need to reply (or communicate privately), but did you send to an OP_RETURN output? It may well be the case that that's why we're seeing something we don't usually see, here. The burn tx functionality was added speculatively but ended up not actually being used in JM. It's possible we should disable it rather than fix this up.

mandelbit commented 2 years ago

Some other config that may be relevant:

running with

blockchain_source=bitcoin-rpc-no-history
mandelbit commented 2 years ago

No need to reply (or communicate privately), but did you send to an OP_RETURN output? It may well be the case that that's why we're seeing something we don't usually see, here. The burn tx functionality was added speculatively but ended up not actually being used in JM. It's possible we should disable it rather than fix this up.

Sent reply via telegram

AdamISZ commented 2 years ago

No need to reply (or communicate privately), but did you send to an OP_RETURN output? It may well be the case that that's why we're seeing something we don't usually see, here. The burn tx functionality was added speculatively but ended up not actually being used in JM. It's possible we should disable it rather than fix this up.

Sent reply via telegram

I pinged you there, think you must have the wrong handle

AdamISZ commented 2 years ago

So first just noting down a couple of things: you're running off version 0.9.2 (which is a bit old! update). I checked the history of the relevant chunks of code in wallet.py and wallet_service.py and it seems they haven't been updated recently, so it's not related to the fact that your code is out of date.

Second, as per earlier discussion, yes seems most likely to be the case where the Bitcoin Core instance sees OP_RETURN outputs in the wallet, but these are not related to Joinmarket, as we indeed have not decided (yet, or possibly ever) to actually use that functionality in this application, even though there is some support for it.

You can see this line:

https://github.com/JoinMarket-Org/joinmarket-clientserver/blame/868e188206b9645437e41801abca256961739126/jmclient/jmclient/wallet_service.py#L663

has not been changed since the introduction of burner outputs, and it's here that we're calling set_next_index without force=True (I haven't yet delved into the precise reasoning around that). At this point maybe @chris-belcher can do a check on whether this should be changed, and in what way, to make sure that if a person happens to have OP_RETURN transactions in their Core wallet it doesn't stop the sync from working?

AdamISZ commented 2 years ago

Ping @chris-belcher just a reminder, could you check that one?