Do not sequence handle updates in slow path (#1292)

* do not sequence handle updates in slow path * build * bail early if invite code doesn't exist
bluesky-social · Jul 6, 2023 · 0d3a555 · 0d3a555
1 parent 3ea892b
commit 0d3a555
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 10 deletions.
diff --git a/.github/workflows/build-and-push-pds-aws.yaml b/.github/workflows/build-and-push-pds-aws.yaml
@@ -3,7 +3,6 @@ on:
   push:
     branches:
       - main
-      - auth-triage-role
 env:
   REGISTRY: ${{ secrets.AWS_ECR_REGISTRY_USEAST2_PACKAGES_REGISTRY }}
   USERNAME: ${{ secrets.AWS_ECR_REGISTRY_USEAST2_PACKAGES_USERNAME }}

diff --git a/packages/pds/src/api/com/atproto/admin/updateAccountHandle.ts b/packages/pds/src/api/com/atproto/admin/updateAccountHandle.ts
@@ -2,7 +2,11 @@ import { AuthRequiredError, InvalidRequestError } from '@atproto/xrpc-server'
 import * as ident from '@atproto/identifier'
 import { Server } from '../../../../lexicon'
 import AppContext from '../../../../context'
-import { UserAlreadyExistsError } from '../../../../services/account'
+import {
+  HandleSequenceToken,
+  UserAlreadyExistsError,
+} from '../../../../services/account'
+import { httpLogger } from '../../../../logger'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.admin.updateAccountHandle({
@@ -48,17 +52,30 @@ export default function (server: Server, ctx: AppContext) {
         throw new InvalidRequestError(`Account not found: ${did}`)
       }
 
-      await ctx.db.transaction(async (dbTxn) => {
+      const seqHandleTok = await ctx.db.transaction(async (dbTxn) => {
+        let tok: HandleSequenceToken
         try {
-          await ctx.services.account(dbTxn).updateHandle(did, handle)
+          tok = await ctx.services.account(dbTxn).updateHandle(did, handle)
         } catch (err) {
           if (err instanceof UserAlreadyExistsError) {
             throw new InvalidRequestError(`Handle already taken: ${handle}`)
           }
           throw err
         }
         await ctx.plcClient.updateHandle(did, ctx.plcRotationKey, handle)
+        return tok
       })
+
+      try {
+        await ctx.db.transaction(async (dbTxn) => {
+          await ctx.services.account(dbTxn).sequenceHandle(seqHandleTok)
+        })
+      } catch (err) {
+        httpLogger.error(
+          { err, did, handle },
+          'failed to sequence handle update',
+        )
+      }
     },
   })
 }
diff --git a/packages/pds/src/api/com/atproto/identity/updateHandle.ts b/packages/pds/src/api/com/atproto/identity/updateHandle.ts
@@ -2,7 +2,11 @@ import { InvalidRequestError } from '@atproto/xrpc-server'
 import * as ident from '@atproto/identifier'
 import { Server } from '../../../../lexicon'
 import AppContext from '../../../../context'
-import { UserAlreadyExistsError } from '../../../../services/account'
+import {
+  HandleSequenceToken,
+  UserAlreadyExistsError,
+} from '../../../../services/account'
+import { httpLogger } from '../../../../logger'
 
 export default function (server: Server, ctx: AppContext) {
   server.com.atproto.identity.updateHandle({
@@ -43,17 +47,32 @@ export default function (server: Server, ctx: AppContext) {
         }
       }
 
-      await ctx.db.transaction(async (dbTxn) => {
+      const seqHandleTok = await ctx.db.transaction(async (dbTxn) => {
+        let tok: HandleSequenceToken
         try {
-          await ctx.services.account(dbTxn).updateHandle(requester, handle)
+          tok = await ctx.services
+            .account(dbTxn)
+            .updateHandle(requester, handle)
         } catch (err) {
           if (err instanceof UserAlreadyExistsError) {
             throw new InvalidRequestError(`Handle already taken: ${handle}`)
           }
           throw err
         }
         await ctx.plcClient.updateHandle(requester, ctx.plcRotationKey, handle)
+        return tok
       })
+
+      try {
+        await ctx.db.transaction(async (dbTxn) => {
+          await ctx.services.account(dbTxn).sequenceHandle(seqHandleTok)
+        })
+      } catch (err) {
+        httpLogger.error(
+          { err, did: requester, handle },
+          'failed to sequence handle update',
+        )
+      }
     },
   })
 }
diff --git a/packages/pds/src/api/com/atproto/server/createAccount.ts b/packages/pds/src/api/com/atproto/server/createAccount.ts
@@ -125,13 +125,20 @@ export const ensureCodeIsAvailable = async (
     .if(withLock && db.dialect === 'pg', (qb) => qb.forUpdate().skipLocked())
     .executeTakeFirst()
 
+  if (!invite || invite.disabled) {
+    throw new InvalidRequestError(
+      'Provided invite code not available',
+      'InvalidInviteCode',
+    )
+  }
+
   const uses = await db.db
     .selectFrom('invite_code_use')
     .select(countAll.as('count'))
     .where('code', '=', inviteCode)
     .executeTakeFirstOrThrow()
 
-  if (!invite || invite.disabled || invite.availableUses <= uses.count) {
+  if (invite.availableUses <= uses.count) {
     throw new InvalidRequestError(
       'Provided invite code not available',
       'InvalidInviteCode',

diff --git a/packages/pds/src/services/account/index.ts b/packages/pds/src/services/account/index.ts
@@ -149,7 +149,12 @@ export class AccountService {
     log.info({ handle, email, did }, 'registered user')
   }
 
-  async updateHandle(did: string, handle: string) {
+  // @NOTE should always be paired with a sequenceHandle().
+  // the token output from this method should be passed to sequenceHandle().
+  async updateHandle(
+    did: string,
+    handle: string,
+  ): Promise<HandleSequenceToken> {
     const res = await this.db.db
       .updateTable('did_handle')
       .set({ handle })
@@ -164,7 +169,12 @@ export class AccountService {
     if (res.numUpdatedRows < 1) {
       throw new UserAlreadyExistsError()
     }
-    const seqEvt = await sequencer.formatSeqHandleUpdate(did, handle)
+    return { did, handle }
+  }
+
+  async sequenceHandle(tok: HandleSequenceToken) {
+    this.db.assertTransaction()
+    const seqEvt = await sequencer.formatSeqHandleUpdate(tok.did, tok.handle)
     await sequencer.sequenceEvt(this.db, seqEvt)
   }
 
@@ -624,3 +634,5 @@ export class ListKeyset extends TimeCidKeyset<{
 const matchNamespace = (namespace: string, fullname: string) => {
   return fullname === namespace || fullname.startsWith(`${namespace}.`)
 }
+
+export type HandleSequenceToken = { did: string; handle: string }