Skip to content

Commit

Permalink
Throw custom errors from relayer (safe-global#1154)
Browse files Browse the repository at this point in the history
This adds custom errors for the following to the relayer:

- Invalid `multiSend`
- Invalid transfers
- Unofficial mastercopies
- Unofficial MultiSend contracts
  • Loading branch information
iamacook authored Feb 19, 2024
1 parent 79938db commit 90374be
Show file tree
Hide file tree
Showing 12 changed files with 223 additions and 35 deletions.
7 changes: 7 additions & 0 deletions src/domain/relay/errors/invalid-multisend.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class InvalidMultiSendError extends Error {
constructor() {
super(
'Invalid multiSend call. The batch is not all execTransaction calls to same address.',
);
}
}
7 changes: 7 additions & 0 deletions src/domain/relay/errors/invalid-transfer.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class InvalidTransferError extends Error {
constructor() {
super(
'Invalid transfer. The proposed transfer is not an execTransaction, multiSend, or createProxyWithNonce call.',
);
}
}
7 changes: 7 additions & 0 deletions src/domain/relay/errors/unofficial-master-copy.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class UnofficialMasterCopyError extends Error {
constructor() {
super(
'Safe attempting to relay is not official. Only official Safe singletons are supported.',
);
}
}
7 changes: 7 additions & 0 deletions src/domain/relay/errors/unofficial-multisend.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class UnofficialMultiSendError extends Error {
constructor() {
super(
'MultiSend contract is not official. Only official MultiSend contracts are supported.',
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Response } from 'express';
import {
Catch,
ExceptionFilter,
ArgumentsHost,
HttpStatus,
} from '@nestjs/common';
import { InvalidMultiSendError } from '@/domain/relay/errors/invalid-multisend.error';

@Catch(InvalidMultiSendError)
export class InvalidMultiSendExceptionFilter implements ExceptionFilter {
catch(error: InvalidMultiSendError, host: ArgumentsHost): void {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();

response.status(HttpStatus.UNPROCESSABLE_ENTITY).json({
message: error.message,
statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Response } from 'express';
import {
Catch,
ExceptionFilter,
ArgumentsHost,
HttpStatus,
} from '@nestjs/common';
import { InvalidTransferError } from '@/domain/relay/errors/invalid-transfer.error';

@Catch(InvalidTransferError)
export class InvalidTransferExceptionFilter implements ExceptionFilter {
catch(error: InvalidTransferError, host: ArgumentsHost): void {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();

response.status(HttpStatus.UNPROCESSABLE_ENTITY).json({
message: error.message,
statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Response } from 'express';
import {
Catch,
ExceptionFilter,
ArgumentsHost,
HttpStatus,
} from '@nestjs/common';
import { UnofficialMasterCopyError } from '@/domain/relay/errors/unofficial-master-copy.error';

@Catch(UnofficialMasterCopyError)
export class UnofficialMasterCopyExceptionFilter implements ExceptionFilter {
catch(_: UnofficialMasterCopyError, host: ArgumentsHost): void {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();

response.status(HttpStatus.UNPROCESSABLE_ENTITY).json({
message: 'Unsupported base contract.',
statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
});
}
}
21 changes: 21 additions & 0 deletions src/domain/relay/exception-filters/unofficial-multisend.error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Response } from 'express';
import {
Catch,
ExceptionFilter,
ArgumentsHost,
HttpStatus,
} from '@nestjs/common';
import { UnofficialMultiSendError } from '@/domain/relay/errors/unofficial-multisend.error';

@Catch(UnofficialMultiSendError)
export class UnofficialMultiSendExceptionFilter implements ExceptionFilter {
catch(_: UnofficialMultiSendError, host: ArgumentsHost): void {
const ctx = host.switchToHttp();
const response = ctx.getResponse<Response>();

response.status(HttpStatus.UNPROCESSABLE_ENTITY).json({
message: 'Unofficial MultiSend contract.',
statusCode: HttpStatus.UNPROCESSABLE_ENTITY,
});
}
}
36 changes: 27 additions & 9 deletions src/domain/relay/limit-addresses.mapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ describe('LimitAddressesMapper', () => {
data,
to: safeAddress,
}),
).rejects.toThrow('Cannot get limit addresses – Invalid transfer');
).rejects.toThrow(
'Invalid transfer. The proposed transfer is not an execTransaction, multiSend, or createProxyWithNonce call.',
);
});

// transfer (execTransaction)
Expand All @@ -335,7 +337,9 @@ describe('LimitAddressesMapper', () => {
data,
to: safeAddress,
}),
).rejects.toThrow('Cannot get limit addresses – Invalid transfer');
).rejects.toThrow(
'Invalid transfer. The proposed transfer is not an execTransaction, multiSend, or createProxyWithNonce call.',
);
});

// Unofficial mastercopy
Expand All @@ -357,7 +361,9 @@ describe('LimitAddressesMapper', () => {
data,
to: safeAddress,
}),
).rejects.toThrow('execTransaction via unofficial Safe mastercopy');
).rejects.toThrow(
'Safe attempting to relay is not official. Only official Safe singletons are supported.',
);
});
});

Expand Down Expand Up @@ -467,7 +473,9 @@ describe('LimitAddressesMapper', () => {
data,
to: getAddress(to),
}),
).rejects.toThrow('Invalid MultiSend transactions');
).rejects.toThrow(
'Invalid multiSend call. The batch is not all execTransaction calls to same address.',
);
});

it('should throw when the mastercopy is not official', async () => {
Expand Down Expand Up @@ -502,7 +510,9 @@ describe('LimitAddressesMapper', () => {
data,
to: getAddress(to),
}),
).rejects.toThrow('multiSend via unofficial Safe mastercopy');
).rejects.toThrow(
'Safe attempting to relay is not official. Only official Safe singletons are supported.',
);
});

it('should throw when the batch is to varying parties', async () => {
Expand Down Expand Up @@ -537,7 +547,9 @@ describe('LimitAddressesMapper', () => {
data,
to: getAddress(to),
}),
).rejects.toThrow('MultiSend transactions target different addresses');
).rejects.toThrow(
'Invalid multiSend call. The batch is not all execTransaction calls to same address.',
);
});

it('should throw for unofficial MultiSend deployments', async () => {
Expand Down Expand Up @@ -571,7 +583,9 @@ describe('LimitAddressesMapper', () => {
data,
to,
}),
).rejects.toThrow('multiSend via unofficial MultiSend contract');
).rejects.toThrow(
'MultiSend contract is not official. Only official MultiSend contracts are supported.',
);
});
});

Expand Down Expand Up @@ -651,7 +665,9 @@ describe('LimitAddressesMapper', () => {
data,
to,
}),
).rejects.toThrow('Cannot get limit addresses – Invalid transfer');
).rejects.toThrow(
'Invalid transfer. The proposed transfer is not an execTransaction, multiSend, or createProxyWithNonce call.',
);
});
});

Expand All @@ -670,7 +686,9 @@ describe('LimitAddressesMapper', () => {
data,
to: safeAddress,
}),
).rejects.toThrow('Cannot get limit addresses – Invalid transfer');
).rejects.toThrow(
'Invalid transfer. The proposed transfer is not an execTransaction, multiSend, or createProxyWithNonce call.',
);
});

it('should throw if the to address is not valid', async () => {
Expand Down
19 changes: 13 additions & 6 deletions src/domain/relay/limit-addresses.mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import {
} from '@safe-global/safe-deployments';
import { SafeDecoder } from '@/domain/contracts/contracts/safe-decoder.helper';
import { isAddress, isHex } from 'viem';
import { UnofficialMasterCopyError } from '@/domain/relay/errors/unofficial-master-copy.error';
import { UnofficialMultiSendError } from '@/domain/relay/errors/unofficial-multisend.error';
import { InvalidTransferError } from '@/domain/relay/errors/invalid-transfer.error';
import { InvalidMultiSendError } from '@/domain/relay/errors/invalid-multisend.error';

@Injectable()
export class LimitAddressesMapper {
Expand Down Expand Up @@ -56,7 +60,7 @@ export class LimitAddressesMapper {
});

if (!isOfficial) {
throw Error('execTransaction via unofficial Safe mastercopy');
throw new UnofficialMasterCopyError();
}

// Safe targeted by execTransaction will be limited
Expand All @@ -71,7 +75,7 @@ export class LimitAddressesMapper {
address: args.to,
})
) {
throw Error('multiSend via unofficial MultiSend contract');
throw new UnofficialMultiSendError();
}

// multiSend calldata meets the validity requirements
Expand All @@ -84,7 +88,7 @@ export class LimitAddressesMapper {
});

if (!isOfficial) {
throw Error('multiSend via unofficial Safe mastercopy');
throw new UnofficialMasterCopyError();
}

// Safe targeted in batch will be limited
Expand All @@ -102,7 +106,7 @@ export class LimitAddressesMapper {
return this.getOwnersFromCreateProxyWithNonce(args.data);
}

throw Error('Cannot get limit addresses – Invalid transfer');
throw new InvalidTransferError();
}

private isValidExecTransactionCall(args: { to: string; data: Hex }): boolean {
Expand Down Expand Up @@ -194,7 +198,7 @@ export class LimitAddressesMapper {
});

if (!isEveryValid) {
throw Error('Invalid MultiSend transactions');
throw new InvalidMultiSendError();
}

const firstRecipient = transactions[0].to;
Expand All @@ -203,8 +207,9 @@ export class LimitAddressesMapper {
return transaction.to === firstRecipient;
});

// Transactions calls execTransaction on varying addresses
if (!isSameRecipient) {
throw Error('MultiSend transactions target different addresses');
throw new InvalidMultiSendError();
}

return firstRecipient;
Expand Down Expand Up @@ -253,6 +258,7 @@ export class LimitAddressesMapper {
});

if (decodedProxyFactory.functionName !== 'createProxyWithNonce') {
// Should never happen but check is needed to satisfy TypeScript
throw Error('Not a createProxyWithNonce call');
}

Expand All @@ -262,6 +268,7 @@ export class LimitAddressesMapper {
});

if (decodedSafe.functionName !== 'setup') {
// No custom error thrown, as caller subsequently throws InvalidTransferError
throw Error('Not a setup call');
}

Expand Down
Loading

0 comments on commit 90374be

Please sign in to comment.