test-recovery functionality error reporting cannot recover valid seed phrase

[infosecual]

I have a mainnet node that provided the following output when I ran rocketpool wallet test-recovery:

user@xxxxx:~$ rocketpool wallet test-recovery

NOTE:
This command will test the recovery of your node wallet's private key and (unless explicitly disabled) the validator keys for your minipools, but will not actually write any files; it's simply a "dry run" of recovery.
Use `rocketpool wallet recover` to actually recover the wallet and validator keys.

Please enter the number of words in your mnemonic phrase (24 by default):
24

Enter Word Number 1 of your mnemonic:
...
...
...
Enter Word Number 24 of your mnemonic:


Testing recovery of node wallet and validator keys...
Could not test recover wallet: attempt limit exceeded (2000 keys)

This led me to think that I had somehow collected the wrong seed phrase when I created the wallet. Because of this I exited all of the node's minipools.

Unsure why this happened I just ran a full recover on a new node (separate hardware, fully synced node w no initialized wallet. This attempt succeeded, showing that my original seed was indeed correct:

...
Recovering node wallet only (ignoring validator keys)...
The node wallet was successfully recovered.
Node account: 0xXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

I believe that the error in the test-recovery functionality is due to the fact that I converted a non-rocketpool node into 4x 8LEBs. I am guessing that the attempt limit exceeded (2000 keys) error comes from looping through the BLS key set unable to find a BLS key that matches one of the converted validators I had in my minipool set.

If I find more time to dig into this issue myself I will but I figured I would leave a gh issue here for now. Note- If this happened to me after a validator conversion I imagine it might happen for people with vanity addresses as well. Obviously The node cannot calculate the BLS keys that it does not have the seed for. However, it would be nice to have better error reporting. I about had a hard attack thinking I had a faulty seed and proceeded to exit double digit pools due to this.

[jshufro]

Hey @infosecual I believe I know why this happens.

When you convert a solo validator into 4x LEB8s, you're actually only making 3 new keypairs. The original keypair has to be imported manually, as described here: https://docs.rocketpool.net/guides/node/solo-staker-migration#optional-step-3-import-the-validator-key

The code behind wallet recover was likely updated when we added support for conversions, so that a node with a minipool registered to it that has keys that were not derived from the mnemonic would work, and wallet test-recovery was likely overlooked (a lack of strong DRY practices have been an ongoing theme).

This relates to #407 , perhaps we can get a two-for-one special on these tasks.

[jshufro]

This theory didn't pan out; the code paths are the same.
You recovered the wallet the second time with --skip-validator-keys (I see (ignoring validator keys)), which is likely why it didn't fail. rocketpool wallet rebuild would have failed with the same message, assuming the bls keys were never imported. Do you remember, when you did the migration, if you left the original bls keys on a validator external to smartnode? That would explain things. Hopefully you set the fee recipient correctly :)

[infosecual]

This happens on the node when trying to rebuild keystores FYI:

user@xxxx:~$ rocketpool wallet b

Rebuilding node validator keystores...
Could not rebuild wallet: attempt limit exceeded (2000 keys)

One of the validators here is a conversion so it will not be able to rebuild

[jshufro]

The code there also accounts for solo conversions, provided the keys are in the custom_keys path.... are they?

[infosecual]

Not sure anymore. I ended up re-providsioning a new node. I have a backup of the .rocketpool dir that I will checkout and report back next time i have access to the machine.