Skip to content

Commit

Permalink
Allow Safe nonce to be a number or string (#2213)
Browse files Browse the repository at this point in the history
Updates the validation schema to allow a number or string, coercing to a number so as to not require changes clientside:

- Allow number/string nonce
- Update tests accordingly
  • Loading branch information
iamacook authored Dec 19, 2024
1 parent 6a10de0 commit 78e5dd6
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 30 deletions.
152 changes: 123 additions & 29 deletions src/domain/safe/entities/schemas/__tests__/safe.schema.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { safeBuilder } from '@/domain/safe/entities/__tests__/safe.builder';
import { SafeSchema } from '@/domain/safe/entities/schemas/safe.schema';
import { faker } from '@faker-js/faker';
import { getAddress } from 'viem';
import { ZodError } from 'zod';

describe('SafeSchema', () => {
it('should validate a valid Safe', () => {
Expand All @@ -12,6 +13,16 @@ describe('SafeSchema', () => {
expect(result.success).toBe(true);
});

it('should coerce a string nonce to a number', () => {
const safe = safeBuilder()
.with('nonce', faker.string.numeric() as unknown as number)
.build();

const result = SafeSchema.safeParse(safe);

expect(result.success && result.data.nonce).toBe(Number(safe.nonce));
});

it.each([
'address' as const,
'masterCopy' as const,
Expand All @@ -30,39 +41,87 @@ describe('SafeSchema', () => {
);
});

['nonce' as const, 'threshold' as const].forEach((field) => {
it(`should allow an integer ${field}`, () => {
const safe = safeBuilder()
.with(field, faker.number.int({ min: 1 }))
.build();
it('should allow an integer nonce', () => {
const safe = safeBuilder()
.with('nonce', faker.number.int({ min: 0 }))
.build();

const result = SafeSchema.safeParse(safe);
const result = SafeSchema.safeParse(safe);

expect(result.success).toBe(true);
});
expect(result.success).toBe(true);
});

it.each([
['boolean', faker.datatype.boolean()],
['undefined', undefined],
['null', null],
['string', faker.string.numeric()],
])(`should not allow a %s ${field}`, (type, value) => {
const safe = safeBuilder()
.with(field, value as unknown as number)
.build();
it.each([
['boolean' as const, faker.datatype.boolean(), undefined],
['undefined' as const, undefined, 'Required'],
['null' as const, null, undefined],
])('should not allow a %s nonce', (type, value, message) => {
const safe = safeBuilder()
.with('nonce', value as unknown as number)
.build();

const result = SafeSchema.safeParse(safe);
const result = SafeSchema.safeParse(safe);

expect(!result.success && result.error.issues).toStrictEqual([
{
code: 'invalid_type',
expected: expect.any(String),
received: type,
path: [field],
message: expect.any(String),
},
]);
});
expect(!result.success && result.error.issues).toStrictEqual([
{
code: 'invalid_union',
unionErrors: [
new ZodError([
{
code: 'invalid_type',
expected: 'number',
received: type,
path: ['nonce'],
message: message ?? `Expected number, received ${type}`,
},
]),
new ZodError([
{
code: 'invalid_type',
expected: 'string',
received: type,
path: ['nonce'],
message: message ?? `Expected string, received ${type}`,
},
]),
],
path: ['nonce'],
message: 'Invalid input',
},
]);
});

it('should allow an integer threshold', () => {
const safe = safeBuilder()
.with('threshold', faker.number.int({ min: 1 }))
.build();

const result = SafeSchema.safeParse(safe);

expect(result.success).toBe(true);
});

it.each([
['boolean', faker.datatype.boolean()],
['undefined', undefined],
['null', null],
['string', faker.string.numeric()],
])('should not allow a %s threshold', (type, value) => {
const safe = safeBuilder()
.with('threshold', value as unknown as number)
.build();

const result = SafeSchema.safeParse(safe);

expect(!result.success && result.error.issues).toStrictEqual([
{
code: 'invalid_type',
expected: expect.any(String),
received: type,
path: ['threshold'],
message: expect.any(String),
},
]);
});

it.each(['owners' as const, 'modules' as const])(
Expand Down Expand Up @@ -127,9 +186,44 @@ describe('SafeSchema', () => {
},
);

it('should not allow optional nonce', () => {
const safe = safeBuilder().build();
// @ts-expect-error nonce is not optional
delete safe.nonce;

const result = SafeSchema.safeParse(safe);

expect(!result.success && result.error.issues).toStrictEqual([
{
code: 'invalid_union',
message: 'Invalid input',
path: ['nonce'],
unionErrors: [
new ZodError([
{
code: 'invalid_type',
expected: 'number',
received: 'undefined',
path: ['nonce'],
message: 'Required',
},
]),
new ZodError([
{
code: 'invalid_type',
expected: 'string',
received: 'undefined',
path: ['nonce'],
message: 'Required',
},
]),
],
},
]);
});

it.each([
'address' as const,
'nonce' as const,
'threshold' as const,
'owners' as const,
'masterCopy' as const,
Expand Down
5 changes: 4 additions & 1 deletion src/domain/safe/entities/schemas/safe.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { z } from 'zod';

export const SafeSchema = z.object({
address: AddressSchema,
nonce: z.number(),
// nonce was changed from a number to string on the the Transaction Service
// @see https://github.com/safe-global/safe-transaction-service/pull/2367
// TODO: only allow strings after Transaction Service is on prod.
nonce: z.union([z.number(), z.string()]).pipe(z.coerce.number()),
threshold: z.number(),
owners: z.array(AddressSchema),
masterCopy: AddressSchema,
Expand Down

0 comments on commit 78e5dd6

Please sign in to comment.