PR comments

src/domain/accounts/counterfactual-safes/counterfactual-safes.repository.interface.ts

@@ -18,7 +18,7 @@ export interface ICounterfactualSafesRepository {
     predictedAddress: `0x${string}`;
   }): Promise<CounterfactualSafe>;
 
-  getOrCreateCounterfactualSafe(args: {
+  upsertCounterfactualSafe(args: {
     authPayload: AuthPayload;
     address: `0x${string}`;
     createCounterfactualSafeDto: CreateCounterfactualSafeDto;

src/domain/accounts/counterfactual-safes/counterfactual-safes.repository.ts

@@ -2,7 +2,10 @@ import { IAccountsRepository } from '@/domain/accounts/accounts.repository.inter
 import { ICounterfactualSafesRepository } from '@/domain/accounts/counterfactual-safes/counterfactual-safes.repository.interface';
 import { CounterfactualSafe } from '@/domain/accounts/counterfactual-safes/entities/counterfactual-safe.entity';
 import { CreateCounterfactualSafeDto } from '@/domain/accounts/counterfactual-safes/entities/create-counterfactual-safe.dto.entity';
-import { AccountDataType } from '@/domain/accounts/entities/account-data-type.entity';
+import {
+  AccountDataType,
+  AccountDataTypeNames,
+} from '@/domain/accounts/entities/account-data-type.entity';
 import { AuthPayload } from '@/domain/auth/entities/auth-payload.entity';
 import { ICounterfactualSafesDatasource } from '@/domain/interfaces/counterfactual-safes.datasource.interface';
 import { LoggingService, ILoggingService } from '@/logging/logging.interface';
@@ -39,16 +42,14 @@ export class CounterfactualSafesRepository
     if (!args.authPayload.isForSigner(args.address)) {
       throw new UnauthorizedException();
     }
-    const [account] = await Promise.all([
-      this.accountsRepository.getAccount({
-        authPayload: args.authPayload,
-        address: args.address,
-      }),
-      this.checkCounterfactualSafesIsEnabled({
-        authPayload: args.authPayload,
-        address: args.address,
-      }),
-    ]);
+    await this.checkCounterfactualSafesIsEnabled({
+      authPayload: args.authPayload,
+      address: args.address,
+    });
+    const account = await this.accountsRepository.getAccount({
+      authPayload: args.authPayload,
+      address: args.address,
+    });
     return this.datasource.getCounterfactualSafe({
       account,
       chainId: args.chainId,
@@ -63,24 +64,22 @@ export class CounterfactualSafesRepository
    * If the Counterfactual Safe exists, it returns it.
    * If the Counterfactual Safe does not exist, it's created.
    */
-  async getOrCreateCounterfactualSafe(args: {
+  async upsertCounterfactualSafe(args: {
     authPayload: AuthPayload;
     address: `0x${string}`;
     createCounterfactualSafeDto: CreateCounterfactualSafeDto;
   }): Promise<CounterfactualSafe> {
     if (!args.authPayload.isForSigner(args.address)) {
       throw new UnauthorizedException();
     }
-    const [account] = await Promise.all([
-      this.accountsRepository.getAccount({
-        authPayload: args.authPayload,
-        address: args.address,
-      }),
-      this.checkCounterfactualSafesIsEnabled({
-        authPayload: args.authPayload,
-        address: args.address,
-      }),
-    ]);
+    await this.checkCounterfactualSafesIsEnabled({
+      authPayload: args.authPayload,
+      address: args.address,
+    });
+    const account = await this.accountsRepository.getAccount({
+      authPayload: args.authPayload,
+      address: args.address,
+    });
 
     try {
       return await this.datasource.getCounterfactualSafe({
@@ -125,11 +124,11 @@ export class CounterfactualSafesRepository
   private async checkCounterfactualSafeDataTypeIsActive(): Promise<AccountDataType> {
     const dataTypes = await this.accountsRepository.getDataTypes();
     const counterfactualSafeDataType = dataTypes.find(
-      (dataType) => dataType.name === 'CounterfactualSafes',
+      (dataType) => dataType.name === AccountDataTypeNames.CounterfactualSafes,
     );
     if (!counterfactualSafeDataType?.is_active) {
       this.loggingService.warn({
-        message: 'CounterfactualSafes data type is not active',
+        message: `${AccountDataTypeNames.CounterfactualSafes} data type is not active`,
       });
       throw new GoneException();
     }

src/domain/accounts/entities/__tests__/account-data-type.builder.ts

@@ -1,11 +1,21 @@
 import { Builder, IBuilder } from '@/__tests__/builder';
-import { AccountDataType } from '@/domain/accounts/entities/account-data-type.entity';
+import {
+  AccountDataType,
+  AccountDataTypeNames,
+} from '@/domain/accounts/entities/account-data-type.entity';
 import { faker } from '@faker-js/faker';
 
 export function accountDataTypeBuilder(): IBuilder<AccountDataType> {
   return new Builder<AccountDataType>()
     .with('id', faker.number.int())
-    .with('name', faker.lorem.slug())
+    .with(
+      'name',
+      faker.helpers.arrayElement([
+        AccountDataTypeNames.CounterfactualSafes,
+        AccountDataTypeNames.AddressBook,
+        AccountDataTypeNames.Watchlist,
+      ]),
+    )
     .with('description', faker.lorem.slug())
     .with('is_active', faker.datatype.boolean())
     .with('created_at', faker.date.recent())

src/domain/accounts/entities/account-data-type.entity.spec.ts

@@ -32,26 +32,42 @@ describe('AccountDataTypeSchema', () => {
     },
   );
 
-  it.each(['name' as const, 'description' as const])(
-    'should not verify an AccountDataType with a integer %s',
-    (field) => {
-      const accountDataType = accountDataTypeBuilder().build();
-      // @ts-expect-error - should be strings
-      accountDataType[field] = faker.number.int();
+  it('should not verify an AccountDataType with a integer description', () => {
+    const accountDataType = accountDataTypeBuilder().build();
+    // @ts-expect-error - should be strings
+    accountDataType['description'] = faker.number.int();
 
-      const result = AccountDataTypeSchema.safeParse(accountDataType);
+    const result = AccountDataTypeSchema.safeParse(accountDataType);
 
-      expect(!result.success && result.error.issues).toStrictEqual([
-        {
-          code: 'invalid_type',
-          expected: 'string',
-          message: 'Expected string, received number',
-          path: [field],
-          received: 'number',
-        },
-      ]);
-    },
-  );
+    expect(!result.success && result.error.issues).toStrictEqual([
+      {
+        code: 'invalid_type',
+        expected: 'string',
+        message: 'Expected string, received number',
+        path: ['description'],
+        received: 'number',
+      },
+    ]);
+  });
+
+  it('should not verify an AccountDataType with a random name', () => {
+    const accountDataType = accountDataTypeBuilder().build();
+    const randomName = faker.string.sample();
+    // @ts-expect-error - should be strings
+    accountDataType['name'] = randomName;
+
+    const result = AccountDataTypeSchema.safeParse(accountDataType);
+
+    expect(!result.success && result.error.issues).toStrictEqual([
+      {
+        code: 'invalid_enum_value',
+        message: `Invalid enum value. Expected 'CounterfactualSafes' | 'AddressBook' | 'Watchlist', received '${randomName}'`,
+        options: ['CounterfactualSafes', 'AddressBook', 'Watchlist'],
+        path: ['name'],
+        received: randomName,
+      },
+    ]);
+  });
 
   it('should not verify an AccountDataType with a non-boolean is_active', () => {
     const accountDataType = accountDataTypeBuilder().build();
@@ -98,7 +114,7 @@ describe('AccountDataTypeSchema', () => {
       },
       {
         code: 'invalid_type',
-        expected: 'string',
+        expected: "'CounterfactualSafes' | 'AddressBook' | 'Watchlist'",
         received: 'undefined',
         path: ['name'],
         message: 'Required',

src/domain/accounts/entities/account-data-type.entity.ts

@@ -1,10 +1,16 @@
 import { RowSchema } from '@/datasources/db/entities/row.entity';
 import { z } from 'zod';
 
+export enum AccountDataTypeNames {
+  CounterfactualSafes = 'CounterfactualSafes',
+  AddressBook = 'AddressBook',
+  Watchlist = 'Watchlist',
+}
+
 export type AccountDataType = z.infer<typeof AccountDataTypeSchema>;
 
 export const AccountDataTypeSchema = RowSchema.extend({
-  name: z.string(),
+  name: z.nativeEnum(AccountDataTypeNames),
   description: z.string().nullish().default(null),
   is_active: z.boolean().default(true),
 });

src/routes/accounts/counterfactual-safes/counterfactual-safes.controller.spec.ts

@@ -23,6 +23,7 @@ import { createCounterfactualSafeDtoBuilder } from '@/domain/accounts/counterfac
 import { accountDataSettingBuilder } from '@/domain/accounts/entities/__tests__/account-data-setting.builder';
 import { accountDataTypeBuilder } from '@/domain/accounts/entities/__tests__/account-data-type.builder';
 import { accountBuilder } from '@/domain/accounts/entities/__tests__/account.builder';
+import { AccountDataTypeNames } from '@/domain/accounts/entities/account-data-type.entity';
 import { authPayloadDtoBuilder } from '@/domain/auth/entities/__tests__/auth-payload-dto.entity.builder';
 import { chainBuilder } from '@/domain/chains/entities/__tests__/chain.builder';
 import { IAccountsDatasource } from '@/domain/interfaces/accounts.datasource.interface';
@@ -101,7 +102,7 @@ describe('CounterfactualSafesController', () => {
       const account = accountBuilder().build();
       const accountDataTypes = [
         accountDataTypeBuilder()
-          .with('name', 'CounterfactualSafes')
+          .with('name', AccountDataTypeNames.CounterfactualSafes)
           .with('is_active', true)
           .build(),
       ];
@@ -134,7 +135,6 @@ describe('CounterfactualSafesController', () => {
           saltNonce: counterfactualSafe.salt_nonce,
           singletonAddress: counterfactualSafe.singleton_address,
           threshold: counterfactualSafe.threshold,
-          accountId: counterfactualSafe.account_id.toString(),
         });
     });
 
@@ -159,14 +159,16 @@ describe('CounterfactualSafesController', () => {
       const address = getAddress(faker.finance.ethereumAddress());
       const chain = chainBuilder().build();
       const counterfactualSafe = counterfactualSafeBuilder().build();
+      const accessToken = 'invalid';
 
       await request(app.getHttpServer())
         .get(
           `/v1/accounts/${address}/storage/counterfactual-safes/${chain.chainId}/${counterfactualSafe.predicted_address}`,
         )
-        .set('Cookie', ['access_token=invalid'])
+        .set('Cookie', [`access_token=${accessToken}`])
         .expect(403);
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt malformed');
       expect(accountsRepository.getAccount).not.toHaveBeenCalled();
       expect(
         counterfactualSafesDataSource.getCounterfactualSafe,
@@ -193,6 +195,7 @@ describe('CounterfactualSafesController', () => {
         .set('Cookie', [`access_token=${accessToken}`])
         .expect(403);
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt not active');
       expect(accountsRepository.getAccount).not.toHaveBeenCalled();
       expect(
         counterfactualSafesDataSource.getCounterfactualSafe,
@@ -212,6 +215,7 @@ describe('CounterfactualSafesController', () => {
         exp: faker.date.past(),
       });
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt expired');
       await request(app.getHttpServer())
         .get(
           `/v1/accounts/${address}/storage/counterfactual-safes/${chain.chainId}/${counterfactualSafe.predicted_address}`,
@@ -226,7 +230,7 @@ describe('CounterfactualSafesController', () => {
     });
 
     it('returns 403 if signer_address is not a valid Ethereum address', async () => {
-      const address = faker.string.hexadecimal() as `0x${string}`;
+      const address = faker.string.sample() as `0x${string}`;
       const chain = chainBuilder().build();
       const counterfactualSafe = counterfactualSafeBuilder().build();
       const authPayloadDto = authPayloadDtoBuilder()
@@ -282,7 +286,7 @@ describe('CounterfactualSafesController', () => {
       const account = accountBuilder().build();
       const accountDataTypes = [
         accountDataTypeBuilder()
-          .with('name', 'CounterfactualSafes')
+          .with('name', AccountDataTypeNames.CounterfactualSafes)
           .with('is_active', true)
           .build(),
       ];
@@ -321,7 +325,7 @@ describe('CounterfactualSafesController', () => {
       const account = accountBuilder().build();
       const accountDataTypes = [
         accountDataTypeBuilder()
-          .with('name', 'CounterfactualSafes')
+          .with('name', AccountDataTypeNames.CounterfactualSafes)
           .with('is_active', true)
           .build(),
       ];
@@ -355,7 +359,6 @@ describe('CounterfactualSafesController', () => {
           saltNonce: counterfactualSafe.salt_nonce,
           singletonAddress: counterfactualSafe.singleton_address,
           threshold: counterfactualSafe.threshold,
-          accountId: counterfactualSafe.account_id.toString(),
         });
 
       expect(
@@ -381,7 +384,7 @@ describe('CounterfactualSafesController', () => {
       const account = accountBuilder().build();
       const accountDataTypes = [
         accountDataTypeBuilder()
-          .with('name', 'CounterfactualSafes')
+          .with('name', AccountDataTypeNames.CounterfactualSafes)
           .with('is_active', true)
           .build(),
       ];
@@ -418,7 +421,6 @@ describe('CounterfactualSafesController', () => {
           saltNonce: counterfactualSafe.salt_nonce,
           singletonAddress: counterfactualSafe.singleton_address,
           threshold: counterfactualSafe.threshold,
-          accountId: counterfactualSafe.account_id.toString(),
         });
 
       expect(
@@ -452,10 +454,12 @@ describe('CounterfactualSafesController', () => {
       const address = getAddress(faker.finance.ethereumAddress());
       const createCounterfactualSafeDto =
         createCounterfactualSafeDtoBuilder().build();
+      const accessToken = 'invalid';
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt malformed');
       await request(app.getHttpServer())
         .put(`/v1/accounts/${address}/storage/counterfactual-safes`)
-        .set('Cookie', ['access_token=invalid'])
+        .set('Cookie', [`access_token=${accessToken}`])
         .send(createCounterfactualSafeDto)
         .expect(403);
 
@@ -482,6 +486,7 @@ describe('CounterfactualSafesController', () => {
         nbf: faker.date.future(),
       });
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt not active');
       await request(app.getHttpServer())
         .put(`/v1/accounts/${address}/storage/counterfactual-safes`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -511,6 +516,7 @@ describe('CounterfactualSafesController', () => {
         exp: faker.date.past(),
       });
 
+      expect(() => jwtService.verify(accessToken)).toThrow('jwt expired');
       await request(app.getHttpServer())
         .put(`/v1/accounts/${address}/storage/counterfactual-safes`)
         .set('Cookie', [`access_token=${accessToken}`])
@@ -527,7 +533,7 @@ describe('CounterfactualSafesController', () => {
     });
 
     it('returns 403 if signer_address is not a valid Ethereum address', async () => {
-      const address = faker.string.hexadecimal() as `0x${string}`;
+      const address = faker.string.sample() as `0x${string}`;
       const chain = chainBuilder().build();
       const createCounterfactualSafeDto =
         createCounterfactualSafeDtoBuilder().build();

src/routes/accounts/counterfactual-safes/counterfactual-safes.controller.ts

@@ -38,13 +38,13 @@ export class CounterfactualSafesController {
   @ApiOkResponse({ type: CounterfactualSafe })
   @Put(':address/storage/counterfactual-safes')
   @UseGuards(AuthGuard)
-  async getOrCreateCounterfactualSafe(
+  async upsertCounterfactualSafe(
     @Auth() authPayload: AuthPayload,
     @Param('address', new ValidationPipe(AddressSchema)) address: `0x${string}`,
     @Body(new ValidationPipe(CreateCounterfactualSafeDtoSchema))
     createCounterfactualSafeDto: CreateCounterfactualSafeDto,
   ): Promise<CounterfactualSafe> {
-    return this.service.getOrCreateCounterfactualSafe({
+    return this.service.upsertCounterfactualSafe({
       authPayload,
       address,
       createCounterfactualSafeDto,

src/routes/accounts/counterfactual-safes/counterfactual-safes.service.ts

@@ -23,13 +23,13 @@ export class CounterfactualSafesService {
     return this.mapCounterfactualSafe(domainCounterfactualSafe);
   }
 
-  async getOrCreateCounterfactualSafe(args: {
+  async upsertCounterfactualSafe(args: {
     authPayload: AuthPayload;
     address: `0x${string}`;
     createCounterfactualSafeDto: CreateCounterfactualSafeDto;
   }): Promise<CounterfactualSafe> {
     const domainCounterfactualSafe =
-      await this.repository.getOrCreateCounterfactualSafe(args);
+      await this.repository.upsertCounterfactualSafe(args);
     return this.mapCounterfactualSafe(domainCounterfactualSafe);
   }
 
@@ -45,7 +45,6 @@ export class CounterfactualSafesService {
       domainCounterfactualSafe.salt_nonce,
       domainCounterfactualSafe.singleton_address,
       domainCounterfactualSafe.threshold,
-      domainCounterfactualSafe.account_id.toString(),
     );
   }
 }