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

Database migration for HMAC Key Records #1377

Closed
wants to merge 3 commits into from
Closed

Conversation

nplasterer
Copy link
Contributor

Part of #422

Adds the database table for hmac key records
Matches the structure currently used for hmac keys in V2 https://github.com/xmtp/proto/blob/main/proto/keystore_api/v1/keystore.proto#L306-L317

@nplasterer nplasterer self-assigned this Dec 4, 2024
@nplasterer nplasterer requested a review from a team as a code owner December 4, 2024 20:16

impl DbConnection {
/// Returns all hmac_key_records for the given group_id
pub fn get_hmac_key_records(
Copy link
Contributor

@mchenani mchenani Dec 4, 2024

Choose a reason for hiding this comment

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

sugesstion: does impl_fetch_list_with_key! the same job?

}

/// Insert hmac_key_records without replacing existing ones
pub fn insert_hmac_key_records(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: can we use impl_fetch_list_with_key!

@mchenani
Copy link
Contributor

mchenani commented Dec 4, 2024

Sorry, mistakenly updated the PR!

@@ -0,0 +1,11 @@
CREATE TABLE "hmac_key_records"(
-- Group ID that the Hmac keys are associated with
"group_id" BLOB NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this based off of the hmac keys work in V2 but @codabrink mentioned we might be doing this based on inboxId now. So maybe this is irrelevant in V3?

@richardhuaaa
Copy link
Contributor

Some high-level thoughts, based on our previous discussion about deriving the hmac key from a user secret rather than a group secret in v3. It might be worth looping @codabrink into this PR too:

  1. Right now the schema stores hmac keys for each thirty_day_periods_since_epoch. However, I think we need to store the user's root key from which the hmac keys are derived, otherwise we can't derive keys for future epochs.
  2. If we are storing the root key, then maybe we don't need to store the derived keys, because we can just derive them when we need them. Then we don't need to worry about storing new records or deleting old records as time progresses.
  3. I think there's an opportunity here to generalize this beyond hmacs. In a nutshell, all of a user's installations need to synchronize a root key used to derive hmacs. We could envision using this same system for other things, for example synchronizing a nickname. I'm wondering if what we want is a generic table for user settings/preferences.

@nplasterer
Copy link
Contributor Author

Closing out for now infavor of a more general database long term for these kinds of preferences similar to our consent records DB.

@nplasterer nplasterer closed this Dec 4, 2024
@nplasterer nplasterer deleted the np/hmac-keys-database branch December 4, 2024 22:43
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.

3 participants