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

Incrementing nextaccount when validator creation fails #1790

Closed
realbigsean opened this issue Oct 19, 2020 · 5 comments
Closed

Incrementing nextaccount when validator creation fails #1790

realbigsean opened this issue Oct 19, 2020 · 5 comments
Labels

Comments

@realbigsean
Copy link
Member

Description

Wallet's nextaccount is incremented even if the validator creation fails.

Version

v0.3.0

Present Behaviour

Creating a new validator while you already have a validator client running fails with Error registering validator 0xa2a41ef2606622de6d6c7e5b1d85789b66e28313f85dd4b63d03892fcbfb082423df064bb4547f9ba265bf77238f703d: SQLError("database is locked"). The validator is not created, but the nextaccount field in the wallet is incremented.

Expected Behaviour

nextaccount should not be incremented. I think failing to create the validator when it isn't registered is a good thing though.

Steps to resolve

I think we'll have to move update(&self.wallet_dir, &self.wallet)?; out of LockedWallet::next_validator and only call it after we've successfully created the new validator keystore.

@paulhauner
Copy link
Member

nextaccount should not be incremented.

IIRC, the validator keystores have been created by this point, so it's a little risky to generate them again (in fact, I think the process will fail if we try to regenerate them).

Perhaps we could try and grab the SQL connection before we generate the validators, then we'd fail even earlier.

@paulhauner paulhauner added the A0 label Nov 9, 2020
@michaelsproul
Copy link
Member

michaelsproul commented Nov 13, 2020

I think we could create an SQLite transaction at the beginning of the process (would require exposing some extra functions), then use the register_validators_in_txn function, and a commit once everything else has succeeded

@michaelsproul
Copy link
Member

I think SQLite will fail as soon as you try to make the transaction, if the database is locked

@realbigsean
Copy link
Member Author

I think we could create an SQLite transaction at the beginning of the process (would require exposing some extra functions), then use the register_validators_in_txn function, and a commit once everything else has succeeded

I tried this out but it doesn't fail until you commit the transaction :/

@michaelsproul
Copy link
Member

We could make an empty transaction and attempt to commit it?

Another user on Discord reported what seems like a similar issue: they imported a key while their VC was running and it created the key but failed to commit the sqlite transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants