From 2107659402037baf1199d24abbbacb239a447ff4 Mon Sep 17 00:00:00 2001 From: Derek Croote Date: Fri, 16 Jun 2023 07:02:40 -0700 Subject: [PATCH] Change the HTTP gateway for ChainAPI test calls (#1794) Changes: - Return data from successful API calls that fail processing - Make reserved parameters inaccessible in pre/post processing --- .changeset/four-dingos-reply.md | 6 + packages/airnode-node/src/api/index.ts | 4 + .../airnode-node/src/api/processing.test.ts | 127 +++++++++++------- packages/airnode-node/src/api/processing.ts | 37 ++++- packages/airnode-node/src/types.ts | 11 +- .../airnode-node/test/e2e/http.feature.ts | 54 ++++++-- packages/airnode-operation/src/server.ts | 4 +- 7 files changed, 171 insertions(+), 72 deletions(-) create mode 100644 .changeset/four-dingos-reply.md diff --git a/.changeset/four-dingos-reply.md b/.changeset/four-dingos-reply.md new file mode 100644 index 0000000000..1a4c0f33b6 --- /dev/null +++ b/.changeset/four-dingos-reply.md @@ -0,0 +1,6 @@ +--- +'@api3/airnode-operation': minor +'@api3/airnode-node': minor +--- + +Change the HTTP gateway for ChainAPI test calls by 1) returning data from successful API calls that fail processing and 2) making reserved parameters inaccessible in pre/post processing diff --git a/packages/airnode-node/src/api/index.ts b/packages/airnode-node/src/api/index.ts index 434b93362f..c48ed7f3a8 100644 --- a/packages/airnode-node/src/api/index.ts +++ b/packages/airnode-node/src/api/index.ts @@ -234,6 +234,10 @@ export async function processSuccessfulApiCall( ); if (!goExtractAndEncodeResponse.success) { const log = logger.pend('ERROR', goExtractAndEncodeResponse.error.message); + // The HTTP gateway is a special case for ChainAPI where we return data from a successful API call that failed processing + if (payload.type === 'http-gateway') { + return [[log], { success: true, errorMessage: goExtractAndEncodeResponse.error.message, data: rawResponse.data }]; + } return [[log], { success: false, errorMessage: goExtractAndEncodeResponse.error.message }]; } diff --git a/packages/airnode-node/src/api/processing.test.ts b/packages/airnode-node/src/api/processing.test.ts index 0d427a9523..f9334c0747 100644 --- a/packages/airnode-node/src/api/processing.test.ts +++ b/packages/airnode-node/src/api/processing.test.ts @@ -1,59 +1,84 @@ import { postProcessApiSpecifications, preProcessApiSpecifications } from './processing'; import * as fixtures from '../../test/fixtures'; -describe('processing', () => { - describe('pre-processing', () => { - it('valid processing code', async () => { - const config = fixtures.buildConfig(); - const preProcessingSpecifications = [ - { - environment: 'Node' as const, - value: 'const output = {...input, from: "ETH"};', - timeoutMs: 5_000, - }, - { - environment: 'Node' as const, - value: 'const output = {...input, newProp: "airnode"};', - timeoutMs: 5_000, - }, - ]; - config.ois[0].endpoints[0] = { ...config.ois[0].endpoints[0], preProcessingSpecifications }; - - const parameters = { _type: 'int256', _path: 'price' }; - const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ parameters }); - - const result = await preProcessApiSpecifications({ type: 'regular', config, aggregatedApiCall }); - - expect(result.aggregatedApiCall.parameters).toEqual({ - _path: 'price', - _type: 'int256', - from: 'ETH', - newProp: 'airnode', - }); +describe('pre-processing', () => { + it('valid processing code', async () => { + const config = fixtures.buildConfig(); + const preProcessingSpecifications = [ + { + environment: 'Node' as const, + value: 'const output = {...input, from: "ETH"};', + timeoutMs: 5_000, + }, + { + environment: 'Node' as const, + value: 'const output = {...input, newProp: "airnode"};', + timeoutMs: 5_000, + }, + ]; + config.ois[0].endpoints[0] = { ...config.ois[0].endpoints[0], preProcessingSpecifications }; + + const parameters = { _type: 'int256', _path: 'price' }; + const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ parameters }); + + const result = await preProcessApiSpecifications({ type: 'regular', config, aggregatedApiCall }); + + expect(result.aggregatedApiCall.parameters).toEqual({ + _path: 'price', + _type: 'int256', + from: 'ETH', + newProp: 'airnode', }); + }); + + it('invalid processing code', async () => { + const config = fixtures.buildConfig(); + const preProcessingSpecifications = [ + { + environment: 'Node' as const, + value: 'something invalid; const output = {...input, from: `ETH`};', + timeoutMs: 5_000, + }, + { + environment: 'Node' as const, + value: 'const output = {...input, newProp: "airnode"};', + timeoutMs: 5_000, + }, + ]; + config.ois[0].endpoints[0] = { ...config.ois[0].endpoints[0], preProcessingSpecifications }; + + const parameters = { _type: 'int256', _path: 'price', from: 'TBD' }; + const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ parameters }); + + const throwingFunc = () => preProcessApiSpecifications({ type: 'regular', config, aggregatedApiCall }); + + await expect(throwingFunc).rejects.toEqual(new Error('SyntaxError: Unexpected identifier')); + }); + + it('makes reserved parameters inaccessible for HTTP gateway requests', async () => { + const config = fixtures.buildConfig(); + const preProcessingSpecifications = [ + { + environment: 'Node' as const, + // pretend the user is trying to 1) override _path and 2) set a new parameter based on + // the presence of the reserved parameter _type (which is inaccessible) + value: 'const output = {...input, from: "ETH", _path: "price.newpath", myVal: input._type ? "123" : "456" };', + timeoutMs: 5_000, + }, + ]; + config.ois[0].endpoints[0] = { ...config.ois[0].endpoints[0], preProcessingSpecifications }; + + const parameters = { _type: 'int256', _path: 'price', to: 'USD' }; + const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ parameters }); + + const result = await preProcessApiSpecifications({ type: 'http-gateway', config, aggregatedApiCall }); - it('invalid processing code', async () => { - const config = fixtures.buildConfig(); - const preProcessingSpecifications = [ - { - environment: 'Node' as const, - value: 'something invalid; const output = {...input, from: `ETH`};', - timeoutMs: 5_000, - }, - { - environment: 'Node' as const, - value: 'const output = {...input, newProp: "airnode"};', - timeoutMs: 5_000, - }, - ]; - config.ois[0].endpoints[0] = { ...config.ois[0].endpoints[0], preProcessingSpecifications }; - - const parameters = { _type: 'int256', _path: 'price', from: 'TBD' }; - const aggregatedApiCall = fixtures.buildAggregatedRegularApiCall({ parameters }); - - const throwingFunc = () => preProcessApiSpecifications({ type: 'regular', config, aggregatedApiCall }); - - await expect(throwingFunc).rejects.toEqual(new Error('SyntaxError: Unexpected identifier')); + expect(result.aggregatedApiCall.parameters).toEqual({ + _path: 'price', // is not overridden + _type: 'int256', + from: 'ETH', // originates from the processing code + to: 'USD', // should be unchanged from the original parameters + myVal: '456', // is set to "456" because _type is not present in the environment }); }); }); diff --git a/packages/airnode-node/src/api/processing.ts b/packages/airnode-node/src/api/processing.ts index e0a2a8cdde..035b2c0b9b 100644 --- a/packages/airnode-node/src/api/processing.ts +++ b/packages/airnode-node/src/api/processing.ts @@ -1,9 +1,28 @@ -import { Endpoint, ProcessingSpecification } from '@api3/ois'; +import { Endpoint, ProcessingSpecification, RESERVED_PARAMETERS } from '@api3/ois'; import { go } from '@api3/promise-utils'; import { unsafeEvaluate, unsafeEvaluateAsync } from './unsafe-evaluate'; import { apiCallParametersSchema } from '../validation'; import { PROCESSING_TIMEOUT } from '../constants'; -import { ApiCallPayload } from '../types'; +import { ApiCallParameters, ApiCallPayload } from '../types'; + +const reservedParameters = RESERVED_PARAMETERS as string[]; + +const removeReservedParameters = (parameters: ApiCallParameters): ApiCallParameters => { + return Object.fromEntries(Object.entries(parameters).filter(([key]) => !reservedParameters.includes(key))); +}; + +/** + * Re-inserts reserved parameters from the initial parameters object into the modified parameters object. + */ +const reInsertReservedParameters = ( + initialParameters: ApiCallParameters, + modifiedParameters: ApiCallParameters +): ApiCallParameters => { + return Object.entries(initialParameters).reduce( + (params, [key, value]) => (reservedParameters.includes(key) ? { ...params, [key]: value } : params), + modifiedParameters + ); +}; export const preProcessApiSpecifications = async (payload: ApiCallPayload): Promise => { const { config, aggregatedApiCall } = payload; @@ -15,6 +34,12 @@ export const preProcessApiSpecifications = async (payload: ApiCallPayload): Prom return payload; } + let inputParameters = aggregatedApiCall.parameters; + // The HTTP gateway is a special case for ChainAPI that is not allowed to access reserved parameters + if (payload.type === 'http-gateway') { + inputParameters = removeReservedParameters(inputParameters); + } + const goProcessedParameters = await go( () => preProcessingSpecifications.reduce(async (input: Promise, currentValue: ProcessingSpecification) => { @@ -26,7 +51,7 @@ export const preProcessApiSpecifications = async (payload: ApiCallPayload): Prom default: throw new Error(`Environment ${currentValue.environment} is not supported`); } - }, Promise.resolve(aggregatedApiCall.parameters)), + }, Promise.resolve(inputParameters)), { retries: 0, totalTimeoutMs: PROCESSING_TIMEOUT } ); @@ -35,7 +60,11 @@ export const preProcessApiSpecifications = async (payload: ApiCallPayload): Prom } // Let this throw if the processed parameters are invalid - const parameters = apiCallParametersSchema.parse(goProcessedParameters.data); + let parameters = apiCallParametersSchema.parse(goProcessedParameters.data); + + if (payload.type === 'http-gateway') { + parameters = reInsertReservedParameters(aggregatedApiCall.parameters, parameters); + } return { ...payload, diff --git a/packages/airnode-node/src/types.ts b/packages/airnode-node/src/types.ts index fd4fbe340b..46c8aaebdb 100644 --- a/packages/airnode-node/src/types.ts +++ b/packages/airnode-node/src/types.ts @@ -223,7 +223,10 @@ export interface AuthorizationByRequestId { } export type RegularApiCallResponse = RegularApiCallSuccessResponse | ApiCallErrorResponse; -export type HttpGatewayApiCallResponse = HttpGatewayApiCallSuccessResponse | ApiCallErrorResponse; +export type HttpGatewayApiCallResponse = + | HttpGatewayApiCallSuccessResponse + | HttpGatewayApiCallPartialResponse + | ApiCallErrorResponse; export type HttpSignedDataApiCallResponse = HttpSignedDataApiCallSuccessResponse | ApiCallErrorResponse; export type ApiCallResponse = RegularApiCallResponse | HttpGatewayApiCallResponse | HttpSignedDataApiCallResponse; @@ -240,6 +243,12 @@ export interface HttpGatewayApiCallSuccessResponse { data: { values: unknown[]; rawValue: unknown; encodedValue: string }; } +export interface HttpGatewayApiCallPartialResponse { + success: true; + errorMessage: string; + data: unknown; +} + export interface HttpSignedDataApiCallSuccessResponse { success: true; data: { timestamp: string; encodedValue: string; signature: string }; diff --git a/packages/airnode-node/test/e2e/http.feature.ts b/packages/airnode-node/test/e2e/http.feature.ts index 857a94cf8a..0366b2767b 100644 --- a/packages/airnode-node/test/e2e/http.feature.ts +++ b/packages/airnode-node/test/e2e/http.feature.ts @@ -4,27 +4,53 @@ import { deployAirnodeAndMakeRequests, increaseTestTimeout } from '../setup/e2e' increaseTestTimeout(); -it('makes a call to test the API', async () => { - const { config } = await deployAirnodeAndMakeRequests(__filename); - +describe('processHttpRequest', () => { const parameters = { from: 'ETH', _type: 'int256', _path: 'result', }; + // EndpointID from the trigger fixture ../fixtures/config/config.ts const endpointId = '0x13dea3311fe0d6b84f4daeab831befbc49e19e6494c41e9e065a09c3c68f43b6'; - const [_err, result] = await processHttpRequest(config, endpointId, parameters); + let config: any; + beforeAll(async () => { + const deploymentData = await deployAirnodeAndMakeRequests(__filename); + config = deploymentData.config; + }); - const expected: HttpGatewayApiCallResponse = { - // Value is returned by the mock server from the operation package - data: { - rawValue: { success: true, result: '723.39202' }, - encodedValue: '0x00000000000000000000000000000000000000000000000000000000044fcf02', - values: ['72339202'], - }, - success: true, - }; - expect(result).toEqual(expected); + it('makes a call to test the API', async () => { + const [_err, result] = await processHttpRequest(config, endpointId, parameters); + + const expected: HttpGatewayApiCallResponse = { + // Value is returned by the mock server from the operation package + data: { + rawValue: { result: '723.39202' }, + encodedValue: '0x00000000000000000000000000000000000000000000000000000000044fcf02', + values: ['72339202'], + }, + success: true, + }; + expect(result).toEqual(expected); + }); + + it('returns data from a successful API call that failed processing', async () => { + const invalidType = 'invalidType'; + + // Use a minimal reserved parameters array with only _type (which is required by OIS) and an invalid value + const modifiedConfig = { ...config }; + modifiedConfig.ois[0].endpoints[0].reservedParameters = [{ name: '_type', fixed: invalidType }]; + const minimalParameters = { from: 'ETH' }; + + const [_err, result] = await processHttpRequest(modifiedConfig, endpointId, minimalParameters); + + const expected: HttpGatewayApiCallResponse = { + data: { result: '723.39202' }, + success: true, + errorMessage: `Invalid type: ${invalidType}`, + }; + + expect(result).toEqual(expected); + }); }); diff --git a/packages/airnode-operation/src/server.ts b/packages/airnode-operation/src/server.ts index 7ca2b74a0d..1c6d8374f8 100644 --- a/packages/airnode-operation/src/server.ts +++ b/packages/airnode-operation/src/server.ts @@ -14,11 +14,11 @@ app.get('/convert', (req, res) => { const { from, to } = req.query; if (from === 'ETH' && to === 'USD') { - res.status(200).send({ success: true, result: '723.39202' }); + res.status(200).send({ result: '723.39202' }); return; } - res.status(404).send({ success: false, error: 'Unknown price pair' }); + res.status(404).send({ error: 'Unknown price pair' }); }); app.listen(PORT, () => {