Need to verify request ID-parameters

[bbenligiray]

Request IDs are derived from request parameters. The derivation changes with type:

Regular:

airnode/packages/protocol/contracts/Airnode.sol

Lines 51 to 55 in 696ebad

	requestId = keccak256(abi.encode(
	noRequests,
	templateId,
	parameters
	));

Short:

airnode/packages/protocol/contracts/Airnode.sol

Lines 100 to 104 in 696ebad

	requestId = keccak256(abi.encode(
	noRequests,
	templateId,
	parameters
	));

Full:

airnode/packages/protocol/contracts/Airnode.sol

Lines 156 to 161 in 696ebad

	requestId = keccak256(abi.encode(
	noRequests,
	providerId,
	endpointId,
	parameters
	));

When the node receives a request, it should verify this derivation to ensure that the Ethereum provider didn't tamper with any of the request parameters.

[bbenligiray]

Example:

airnode/packages/protocol/test/UserFlow.js

Lines 212 to 219 in 696ebad

	// Verify that the request parameters are not tampered with
	const expectedRequestId = ethers.utils.keccak256(
	ethers.utils.defaultAbiCoder.encode(
	['uint256', 'bytes32', 'bytes'],
	[parsedRequestLog.args.noRequests, parsedRequestLog.args.templateId, parsedRequestLog.args.parameters]
	)
	);
	expect(parsedRequestLog.args.requestId).to.equal(expectedRequestId);

Would be slightly different for a full request

[andreogle]

Just to be clear, should this validate withdrawals too?

[bbenligiray]

No, the node already provides all the withdrawal request parameters while fulfilling it, so we can do the check on-chain

airnode/packages/protocol/contracts/ProviderStore.sol

Lines 130 to 139 in c17870b

	bytes32 withdrawalParameters = keccak256(abi.encodePacked(
	providerId,
	requesterInd,
	msg.sender,
	destination
	));
	require(
	withdrawalRequestIdToParameters[withdrawalRequestId] == withdrawalParameters,
	"No such withdrawal request"
	);

In theory, we can also do the check for API requests on-chain, but it would require the node to send all the request parameters back (parameters for example) during fulfillment, which would create quite the gas cost overhead. So there is a somewhat arbitrary tradeoff decision that I made here.