fix(server): initialize genesis repo when creating an account with an existing did - #122
Open
edmundedgar wants to merge 1 commit into
Open
Conversation
handleCreateAccount only committed an empty MST and announced the new identity (#identity, #sync) inside `if request.Did == nil`, i.e. only when cocoon minted the DID itself. Accounts created by passing an existing DID (the self-custodied-rotation-key flow: mint your own did:plc, then create the account against it with a service-auth JWT) got a Repo/Actor row but no commit — rev stayed "" and root stayed NULL forever, since nothing else ever initializes it. Any subsequent write (e.g. creating a post) failed with an "invalid cid" error trying to build on a nonexistent repo state, and the account was never announced via #identity/#sync on the firehose (only #account, later, from activateAccount) — see the defensive `if len(urepo.Repo.Root) > 0` guard already in handleServerActivateAccount before it emits #sync, which was silently no-op'ing for exactly this reason. Extracted the commit-and-record logic into initializeGenesisRepo and call it unconditionally for every new account rather than only when cocoon minted the DID. Confirmed safe with respect to the repo-import flow: handleRepoImportRepo unconditionally overwrites root/rev from the uploaded CAR regardless of prior state, so an empty genesis commit doesn't conflict with a later importRepo call. Test (server/handle_server_create_account_test.go): - TestCreateAccountInitializesRepoForExistingDID — creates an account via the existing-DID path (mints a k256 key, seeds a passport cache with a matching DID doc so validateServiceAuth resolves it without a network call, signs a service-auth JWT, POSTs createAccount), then asserts rev/root are populated and both #identity and #sync are in event_records for the DID. Confirmed this test is actually catching the bug: temporarily re-added the old `if request.Did == nil` gate around just the initializeGenesisRepo call, watched the test fail with "repo has no rev after account creation via the existing-DID flow", then reverted. go vet ./... and go test -race ./... pass; gofmt -l . is clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A new account needs initialization with a genesis repo commit before you can write to it. This was correctly done when the PDS creates the account along with a brand new DID, and also when you imported an account from another PDS. However, if you created the DID for a new account yourself, then called
createAccount,rev/rootwould stay""/NULLforever.Motivation
I want to do this as part of a flow where you create a fully self-custodial account on the client side and the only rotation key in the history is custodied by the user, never the PDS.
How it works
The commit-and-record logic that used to run only inside
if request.Did == nilis extracted intoinitializeGenesisRepoand now called unconditionally. This shouldn't affectimportRepowhich unconditionally overwritesroot/revfrom the uploaded CAR regardless of prior state, so an empty genesis commit doesn't conflict with a later import.Surface
server/handle_server_create_account.go: newinitializeGenesisRepo(ctx, did, signingKey) (cid.Cid, string, error),handleCreateAccountcalls it unconditionally instead of only whenrequest.Did == nil.server/handle_server_create_account_test.go: new test.Tests
TestCreateAccountInitializesRepoForExistingDIDcreates an account via the existing-DID path and assertsrev/rootare populated and both#identityand#syncevents are recorded. Confirmed it catches the bug: temporarily restored the oldif request.Did == nilgate, watched it fail, reverted.go vet ./...andgo test -race ./...pass;gofmt -l .is clean.