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

vspadmin: Add retirexpub command #480

Merged
merged 4 commits into from
Jun 27, 2024
Merged

Conversation

jholdstock
Copy link
Member

Database Upgrade Warning

This PR adds a new command retirexpub to vspadmin. It opens an existing vspd database and replaces the currently used xpub with a new one. A database upgrade was required in order to support this functionality.

Current and historic xpubs are displayed on a new tab in the admin page of the web UI.

Closes #461

**Warning: This commit contains a database upgrade.**

In order to add future support for retiring xpub keys, the database is
upgraded such that the keys are now stored in a dedicated bucket which
can hold multiple values rather than storing a single key as individual
values in the root bucket.

A new ID field is added to distinguish between keys. This ID is added to
every ticket record in the database in order to track which pubkey was
used for each ticket.

A new field named "Retired" has also been added to pubkeys. It is a unix
timestamp representing the moment the key was retired, or zero for the
currently active key.
A new tab is added to the admin page to display current and historic
xpub keys used by vspd.
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice job overall! I appreciate the good code documentation to help assert it's doing what was intended and addition of solid test coverage.

cmd/vspadmin/main.go Outdated Show resolved Hide resolved
}
}

return xpubs[highest], nil
Copy link
Member

Choose a reason for hiding this comment

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

I believe the initialization of the database ensures there is at least one xpub, so it probably doesn't matter, but this will crash if there are no xpubs in the database. It seems like it'd be safer to me to have a check before the loop like:

if len(xpubs) == 0 {
	return FeeXPub{}, /* no xpub configured error */
}

// Find the active xpub - the one with the highest ID.
var highest uint32
...

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we already have that scenario covered - AllXPubs() will return a "bucket does not exist" error if there are no xpubs in the DB.

Copy link
Member

Choose a reason for hiding this comment

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

That's what I meant by the bit about saying I believe the initialization of the database ensures there is at least one. Generally speaking, just because a bucket exists does not necessarily imply it has anything in it. It probably doesn't matter too much for vspd, but I'm always super careful about such things in dcrd to avoid crashes.

The new command opens an existing vspd database and replaces the
currently used xpub with a new one.
@jholdstock jholdstock merged commit cab4058 into decred:master Jun 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hot-swapping the fees xpub on an existing VSP
2 participants