Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove airnode address from template verification #1037

Merged
merged 4 commits into from
Apr 27, 2022
Merged

Remove airnode address from template verification #1037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3522c7b
Remove airnode address from templateId derivation
vponline Apr 26, 2022
3db6106
Add changeset
vponline Apr 26, 2022
5877c94
Add separate derivation functions for V0 and V1 contracts
vponline Apr 26, 2022
3bbc077
Merge branch 'master' into signed-gateway-template-id-fix
vponline Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stale-beers-tickle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@api3/airnode-node': patch
---

Remove airnode address from templateId verification
10 changes: 7 additions & 3 deletions packages/airnode-node/src/api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getMasterHDNode } from '../evm';
import { getReservedParameters } from '../adapters/http/parameters';
import { API_CALL_TIMEOUT, API_CALL_TOTAL_TIMEOUT } from '../constants';
import { isValidSponsorWallet, isValidRequestId } from '../evm/verification';
import { getExpectedTemplateId } from '../evm/templates';
import { getExpectedTemplateIdV0, getExpectedTemplateIdV1 } from '../evm/templates';
import { AggregatedApiCall, ApiCallResponse, LogsData, RequestErrorMessage, ApiCallErrorResponse } from '../types';
import { Config } from '../config/types';

Expand Down Expand Up @@ -126,7 +126,7 @@ function verifyRequestId(payload: CallApiPayload): LogsData<ApiCallErrorResponse

export function verifyTemplateId(payload: CallApiPayload): LogsData<ApiCallErrorResponse> | null {
const { aggregatedApiCall } = payload;
// TODO: check if beacon needs to verify templates

if (aggregatedApiCall.type === 'http-gateway') return null;

const { templateId, template, id } = aggregatedApiCall;
Expand All @@ -146,7 +146,11 @@ export function verifyTemplateId(payload: CallApiPayload): LogsData<ApiCallError
];
}

const expectedTemplateId = getExpectedTemplateId(template);
const expectedTemplateId =
aggregatedApiCall.type === 'http-signed-data-gateway'
? getExpectedTemplateIdV1(template)
: getExpectedTemplateIdV0(template);

if (templateId === expectedTemplateId) return null;

const message = `Invalid template ID:${templateId} found for Request:${id}. Expected template ID:${expectedTemplateId}`;
Expand Down
32 changes: 31 additions & 1 deletion packages/airnode-node/src/evm/templates/template-verification.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { utils } from 'ethers';
import * as verification from './template-verification';
import * as fixtures from '../../../test/fixtures';
import { verifyTemplateId } from '../../api';
Expand All @@ -12,6 +13,35 @@ describe('verify', () => {

const TEMPLATE_ID = '0xb2f063157fcc3c986daf4c2cf1b8ac8b8843f2b1a54c5de5e1ebdf12fb85a927';

it('derives templateId for V0', () => {
const expectedTemplateIdV0 = verification.getExpectedTemplateIdV0(validTemplateFields);

expect(expectedTemplateIdV0).toEqual(
utils.keccak256(
utils.solidityPack(
['address', 'bytes32', 'bytes'],
[
'0x5FbDB2315678afecb367f032d93F642f64180aa3',
'0x2f3a3adf6daf5a3bb00ab83aa82262a6a84b59b0a89222386135330a1819ab48',
'0x6466726f6d63455448',
]
)
)
);
});
it('derives templateId for V1', () => {
const expectedTemplateIdV1 = verification.getExpectedTemplateIdV1(validTemplateFields);

expect(expectedTemplateIdV1).toEqual(
utils.keccak256(
utils.solidityPack(
['bytes32', 'bytes'],
['0x2f3a3adf6daf5a3bb00ab83aa82262a6a84b59b0a89222386135330a1819ab48', '0x6466726f6d63455448']
)
)
);
});

it('returns API calls not linked to templates', () => {
const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ templateId: null });
const config = fixtures.buildConfig();
Expand Down Expand Up @@ -79,7 +109,7 @@ describe('verify', () => {
templateId: TEMPLATE_ID,
template: invalidTemplate,
});
const expectedTemplateId = verification.getExpectedTemplateId(invalidTemplate);
const expectedTemplateId = verification.getExpectedTemplateIdV0(invalidTemplate);
const response = verifyTemplateId({ aggregatedApiCall, config });
expect(response).toEqual([
[
Expand Down
24 changes: 21 additions & 3 deletions packages/airnode-node/src/evm/templates/template-verification.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,34 @@ interface ValidatedField {
readonly value: any;
}

function getTemplateIdValidationFields(template: ApiCallTemplateWithoutId): ValidatedField[] {
function getTemplateIdValidationFieldsV0(template: ApiCallTemplateWithoutId): ValidatedField[] {
return [
{ type: 'address', value: template.airnodeAddress },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @acenolaza is correct. The getExpectedTemplateId function is used in api/index.ts (https://github.com/api3dao/airnode/blob/master/packages/airnode-node/src/api/index.ts#L149) both for signed data gateway requests but also regular Airnode RRP requests. That's why you had to change tests everywhere, not just the ones for callApi. I think this change breaks the Airnode RRP. We need two different template ID derivation functions, one for Airnode RRP (v0 contracts) and one for signed data gateway (v1 contracts).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you prefer to have two separate functions, or we could pass the request type (regular/http-signed-gateway) into getExpectedTemplateId and return based on that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think two separate functions will result in a nicer code.

{ type: 'bytes32', value: template.endpointId },
{ type: 'bytes', value: template.encodedParameters },
];
}

export function getExpectedTemplateId(template: ApiCallTemplateWithoutId): string {
const validatedFields = getTemplateIdValidationFields(template);
export function getExpectedTemplateIdV0(template: ApiCallTemplateWithoutId): string {
const validatedFields = getTemplateIdValidationFieldsV0(template);
const types = validatedFields.map((v) => v.type);
const values = validatedFields.map((v) => v.value);
const {
utils: { keccak256, solidityPack },
} = ethers;

return keccak256(solidityPack(types, values));
}

function getTemplateIdValidationFieldsV1(template: ApiCallTemplateWithoutId): ValidatedField[] {
return [
{ type: 'bytes32', value: template.endpointId },
{ type: 'bytes', value: template.encodedParameters },
];
}

export function getExpectedTemplateIdV1(template: ApiCallTemplateWithoutId): string {
const validatedFields = getTemplateIdValidationFieldsV1(template);
const types = validatedFields.map((v) => v.type);
const values = validatedFields.map((v) => v.value);
const {
Expand Down
2 changes: 1 addition & 1 deletion packages/airnode-node/src/handlers/process-http-signed-data-request.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as fixtures from '../../test/fixtures';
import { HttpSignedDataApiCallSuccessResponse } from '../types';

const ENDPOINT_ID = '0x13dea3311fe0d6b84f4daeab831befbc49e19e6494c41e9e065a09c3c68f43b6';
const TEMPLATE_ID = '0x600975681b98422eee1146d4b835a8103689ae4cddb76069925a929caf0eb79f';
const TEMPLATE_ID = '0xaa1525fe964092a826934ff09c75e1db395b947543a4ca3eb4a19628bad6c6d5';

function buildConfigWithEndpoint(endpoint?: Endpoint) {
const endpoints = endpoint ? [endpoint] : [];
Expand Down
4 changes: 2 additions & 2 deletions packages/airnode-node/src/handlers/process-http-signed-data-request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as evm from '../evm';
import { AggregatedApiCall, HttpSignedDataApiCallSuccessResponse, ApiCallTemplateWithoutId } from '../types';
import { callApi } from '../api';
import { Config } from '../config/types';
import { getExpectedTemplateId } from '../evm/templates';
import { getExpectedTemplateIdV1 } from '../evm/templates';

export async function processHttpSignedDataRequest(
config: Config,
Expand Down Expand Up @@ -39,7 +39,7 @@ export async function processHttpSignedDataRequest(
endpointId,
encodedParameters,
};
const templateId = getExpectedTemplateId(template);
const templateId = getExpectedTemplateIdV1(template);

const aggregatedApiCall: AggregatedApiCall = {
type: 'http-signed-data-gateway',
Expand Down