From 87a156ec6a18e382e9fe0b9498aa2aa44333edcf Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Wed, 12 Apr 2023 10:50:38 +0300 Subject: [PATCH 1/7] Refer to AirnodeRrpV0 with its name in the scripts --- packages/airnode-protocol/deploy/1_deploy.js | 4 ++-- packages/airnode-protocol/deploy/2_verify.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/airnode-protocol/deploy/1_deploy.js b/packages/airnode-protocol/deploy/1_deploy.js index 535d896657..38e6c37363 100644 --- a/packages/airnode-protocol/deploy/1_deploy.js +++ b/packages/airnode-protocol/deploy/1_deploy.js @@ -19,11 +19,11 @@ module.exports = async ({ getUnnamedAccounts, deployments }) => { }); log(`Deployed RequesterAuthorizerWithAirnode at ${requesterAuthorizerWithAirnode.address}`); - const airnodeRrp = await deploy('AirnodeRrpV0', { + const airnodeRrpV0 = await deploy('AirnodeRrpV0', { from: accounts[0], log: true, deterministicDeployment: process.env.DETERMINISTIC ? hre.ethers.constants.HashZero : undefined, }); - log(`Deployed Airnode RRP at ${airnodeRrp.address}`); + log(`Deployed AirnodeRrpV0 at ${airnodeRrpV0.address}`); }; module.exports.tags = ['deploy']; diff --git a/packages/airnode-protocol/deploy/2_verify.js b/packages/airnode-protocol/deploy/2_verify.js index d581c4f2d2..45f89829e3 100644 --- a/packages/airnode-protocol/deploy/2_verify.js +++ b/packages/airnode-protocol/deploy/2_verify.js @@ -13,9 +13,9 @@ module.exports = async ({ deployments }) => { constructorArguments: [AccessControlRegistry.address, 'RequesterAuthorizerWithAirnode admin'], }); - const AirnodeRrp = await deployments.get('AirnodeRrpV0'); + const AirnodeRrpV0 = await deployments.get('AirnodeRrpV0'); await hre.run('verify:verify', { - address: AirnodeRrp.address, + address: AirnodeRrpV0.address, constructorArguments: [], }); }; From b61a581e2c0aa7c2bae72dc6bde4f46b6c1da235 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Wed, 12 Apr 2023 12:13:22 +0300 Subject: [PATCH 2/7] Disable the solhint rule added with a version bump --- .solhint.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.solhint.json b/.solhint.json index 966b74f287..da7fa7aab2 100644 --- a/.solhint.json +++ b/.solhint.json @@ -4,6 +4,7 @@ "compiler-version": "off", "func-visibility": ["warn", { "ignoreConstructors": true }], "not-rely-on-time": "off", - "no-empty-blocks": "off" + "no-empty-blocks": "off", + "no-global-import": "off" } } From 14f7d00b2d810916973904462f6d2a6691a3f6ac Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Wed, 12 Apr 2023 12:57:34 +0300 Subject: [PATCH 3/7] Add AirnodeRrpV0DryRun --- .../contracts/rrp/AirnodeRrpV0DryRun.sol | 110 +++++++++++ .../test/rrp/AirnodeRrpV0DryRun.sol.js | 176 ++++++++++++++++++ 2 files changed, 286 insertions(+) create mode 100644 packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol create mode 100644 packages/airnode-protocol/test/rrp/AirnodeRrpV0DryRun.sol.js diff --git a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol new file mode 100644 index 0000000000..4c80ab9ed2 --- /dev/null +++ b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.9; + +import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; + +/// @title Contract that complements Airnode request–response protocol (RRP) to +/// allow Airnode to estimate the gas required to execute a fulfillment +/// @dev Typically, contracts are built to revert when an external call they +/// make reverts. In contrast, AirnodeRrpV0 does not revert when the external +/// call during the fulfillment reverts, and instead fails gracefully by +/// emitting a `FailedRequest` event. This event signals to the future +/// invocations of the stateless Airnode to not retry the failed fulfillment. +/// Although this approach meets the intended purpose, it disables Airnode from +/// calling `eth_estimateGas` on `fulfill()` to estimate the gas amount that +/// will be used to execute a fulfillment successfully. Specifically, since +/// `eth_estimateGas` looks for the lowest gas limit that results in the +/// transaction not reverting, and AirnodeRrpV0's `fulfill()` does not revert +/// when its external call reverts (because it runs out of gas), +/// `eth_estimateGas` will not necessarily return a gas amount that will result +/// in the fulfillment to be successful even if such an amount exists. +/// As a solution, Airnode calls `eth_estimateGas` on AirnodeRrpV0DryRun's +/// `fulfill()` and the external call of the fulfillment, and add these up to +/// find the gas limit required to execute a successful fulfillment. This +/// sum is an overestimation of the actual requirement, as it includes an +/// additional base fee (21,000 gas on Ethereum). +contract AirnodeRrpV0DryRun +{ + using ECDSA for bytes32; + + event FulfilledRequest( + address indexed airnode, + bytes32 indexed requestId, + bytes data + ); + + // It is virtually impossible for a contract to be deployed at this address + address private constant ADDRESS_WITH_NO_BYTECODE = address(uint160(uint256(keccak256("Address with no bytecode")))); + + /// @dev This mapping is kept as it is in AirnodeRrpV0 to closely simulate + /// the fulfillment. All of its keys will map to zero values. + mapping(bytes32 => bytes32) private requestIdToFulfillmentParameters; + + // solhint-disable-next-line payable-fallback + fallback() external { } + + /// @notice Used by Airnode to estimate the gas amount needed to fulfill + /// the request (excluding the external call). Do not call this function, + /// as it will have no practical effect. + /// @dev Refer to AirnodeRrpV0's `fulfill()` for more information + /// @param requestId Request ID + /// @param airnode Airnode address + /// @param data Fulfillment data + /// @param fulfillAddress Address that will be called to fulfill + /// @param fulfillFunctionId Signature of the function that will be called + /// to fulfill + /// @return callSuccess If the fulfillment call succeeded + /// @return callData Data returned by the fulfillment call (if there is + /// any) + function fulfill( + bytes32 requestId, + address airnode, + address fulfillAddress, + bytes4 fulfillFunctionId, + bytes calldata data, + bytes calldata signature + ) external returns (bool callSuccess, bytes memory callData) { + // The line below is kept the same, except that the condition is + // reversed to ensure that it never reverts. All + // `requestIdToFulfillmentParameters` values are zero and virtually any + // `keccak256()` output will not be equal to that. + require( + keccak256( + abi.encodePacked( + airnode, + msg.sender, + fulfillAddress, + fulfillFunctionId + ) + ) != requestIdToFulfillmentParameters[requestId], + "Dummy revert string" + ); + // The line below does not need to be modified + require( + ( + keccak256(abi.encodePacked(requestId, data)) + .toEthSignedMessageHash() + ).recover(signature) == airnode, + "Invalid signature" + ); + // We cannot call `fulfillAddress` below because (1) we do not want + // this function to actually fulfill the request (2) the fulfill + // function will be behind an `onlyAirnodeRrp` modifier and will reject + // the calls from AirnodeRrpV0DryRun. + // Instead, we call an address that we know to not contain any + // bytecode, which means this call will not revert or spend extra gas. + (callSuccess, callData) = ADDRESS_WITH_NO_BYTECODE.call( // solhint-disable-line avoid-low-level-calls + abi.encodeWithSelector(fulfillFunctionId, requestId, data) + ); + // If the external call above does not succeed, the `eth_estimateGas` + // called on the external call will not be able to return a gas amount. + // AirnodeRrpV0DryRun's `fulfill()` optimistically estimates the + // AirnodeRrpV0 overhead of a fulfillment, and expects Airnode to + // detect if the external call will succeed (by calling + // `eth_estimateGas` on it) independently. Therefore, we do not need to + // consider the unhappy path here. + if (callSuccess) { + emit FulfilledRequest(airnode, requestId, data); + } + } +} diff --git a/packages/airnode-protocol/test/rrp/AirnodeRrpV0DryRun.sol.js b/packages/airnode-protocol/test/rrp/AirnodeRrpV0DryRun.sol.js new file mode 100644 index 0000000000..407139a692 --- /dev/null +++ b/packages/airnode-protocol/test/rrp/AirnodeRrpV0DryRun.sol.js @@ -0,0 +1,176 @@ +const hre = require('hardhat'); +const { expect } = require('chai'); +const utils = require('../utils'); + +describe('AirnodeRrpV0DryRun', () => { + let roles; + let airnodeRrp, airnodeRrpDryRun, rrpRequester; + let airnodeAddress, airnodeMnemonic, airnodeXpub, airnodeWallet; + let sponsorWalletAddress; + + beforeEach(async () => { + const accounts = await hre.ethers.getSigners(); + roles = { + deployer: accounts[0], + sponsor: accounts[1], + randomPerson: accounts[9], + }; + const airnodeRrpFactory = await hre.ethers.getContractFactory('AirnodeRrpV0', roles.deployer); + airnodeRrp = await airnodeRrpFactory.deploy(); + const airnodeRrpDryRunFactory = await hre.ethers.getContractFactory('AirnodeRrpV0DryRun', roles.deployer); + airnodeRrpDryRun = await airnodeRrpDryRunFactory.deploy(); + const rrpRequesterFactory = await hre.ethers.getContractFactory('MockRrpRequesterV0', roles.deployer); + rrpRequester = await rrpRequesterFactory.deploy(airnodeRrp.address); + ({ airnodeAddress, airnodeMnemonic, airnodeXpub } = utils.generateRandomAirnodeWallet()); + airnodeWallet = hre.ethers.Wallet.fromMnemonic(airnodeMnemonic, "m/44'/60'/0'/0/0"); + sponsorWalletAddress = utils.deriveSponsorWalletAddress(airnodeXpub, roles.sponsor.address); + await roles.deployer.sendTransaction({ + to: airnodeAddress, + value: hre.ethers.utils.parseEther('1'), + }); + await roles.deployer.sendTransaction({ + to: sponsorWalletAddress, + value: hre.ethers.utils.parseEther('1'), + }); + }); + + describe('fulfill', function () { + context('Signature is valid', function () { + it('returns `true` and emits FulfilledRequest', async function () { + const endpointId = utils.generateRandomBytes32(); + const requestTimeParameters = utils.generateRandomBytes(); + const requestId = hre.ethers.utils.keccak256( + hre.ethers.utils.solidityPack( + [ + 'uint256', + 'address', + 'address', + 'uint256', + 'address', + 'bytes32', + 'address', + 'address', + 'address', + 'bytes4', + 'bytes', + ], + [ + (await hre.ethers.provider.getNetwork()).chainId, + airnodeRrp.address, + rrpRequester.address, + (await airnodeRrp.requesterToRequestCountPlusOne(rrpRequester.address)).sub(1), + airnodeAddress, + endpointId, + roles.sponsor.address, + sponsorWalletAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + requestTimeParameters, + ] + ) + ); + const fulfillData = hre.ethers.utils.defaultAbiCoder.encode(['uint256', 'string'], ['123456', 'hello']); + const signature = await airnodeWallet.signMessage( + hre.ethers.utils.arrayify( + hre.ethers.utils.keccak256(hre.ethers.utils.solidityPack(['bytes32', 'bytes'], [requestId, fulfillData])) + ) + ); + const staticCallResult = await airnodeRrpDryRun.callStatic.fulfill( + requestId, + airnodeAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + fulfillData, + signature, + { gasLimit: 500000 } + ); + expect(staticCallResult.callSuccess).to.equal(true); + expect(staticCallResult.callData).to.equal('0x'); + await expect( + airnodeRrpDryRun.fulfill( + requestId, + airnodeAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + fulfillData, + signature, + { gasLimit: 500000 } + ) + ) + .to.emit(airnodeRrpDryRun, 'FulfilledRequest') + .withArgs(airnodeAddress, requestId, fulfillData); + const estimatedGas = await airnodeRrpDryRun.estimateGas.fulfill( + requestId, + airnodeAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + fulfillData, + signature + ); + // The estimated gas will be ~40,000. This number can slightly change + // with future forks, and differ greatly between roll-ups and regular + // chains. + expect(estimatedGas.toNumber()).to.greaterThan(30000); + expect(estimatedGas.toNumber()).to.lessThan(50000); + }); + }); + context('Signature is not valid', function () { + it('reverts', async function () { + const endpointId = utils.generateRandomBytes32(); + const requestTimeParameters = utils.generateRandomBytes(); + const requestId = hre.ethers.utils.keccak256( + hre.ethers.utils.solidityPack( + [ + 'uint256', + 'address', + 'address', + 'uint256', + 'address', + 'bytes32', + 'address', + 'address', + 'address', + 'bytes4', + 'bytes', + ], + [ + (await hre.ethers.provider.getNetwork()).chainId, + airnodeRrp.address, + rrpRequester.address, + (await airnodeRrp.requesterToRequestCountPlusOne(rrpRequester.address)).sub(1), + airnodeAddress, + endpointId, + roles.sponsor.address, + sponsorWalletAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + requestTimeParameters, + ] + ) + ); + const fulfillData = hre.ethers.utils.defaultAbiCoder.encode(['uint256', 'string'], ['123456', 'hello']); + await expect( + airnodeRrpDryRun.fulfill( + requestId, + airnodeAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + fulfillData, + '0x123456', + { gasLimit: 500000 } + ) + ).to.be.revertedWith('ECDSA: invalid signature length'); + await expect( + airnodeRrpDryRun.estimateGas.fulfill( + requestId, + airnodeAddress, + rrpRequester.address, + rrpRequester.interface.getSighash('fulfill'), + fulfillData, + '0x123456' + ) + ).to.be.revertedWith('ECDSA: invalid signature length'); + }); + }); + }); +}); From 134b6ff215595167ff084916f66b51eeb718a456 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Wed, 12 Apr 2023 12:59:12 +0300 Subject: [PATCH 4/7] Add changeset --- .changeset/thick-pandas-battle.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thick-pandas-battle.md diff --git a/.changeset/thick-pandas-battle.md b/.changeset/thick-pandas-battle.md new file mode 100644 index 0000000000..e7862b035c --- /dev/null +++ b/.changeset/thick-pandas-battle.md @@ -0,0 +1,5 @@ +--- +'@api3/airnode-protocol': minor +--- + +Add AirnodeRrpV0DryRun.sol From 2121175ba698fd480808c097b4393fdd198e1055 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Thu, 13 Apr 2023 16:11:53 +0300 Subject: [PATCH 5/7] Remove obsolete fallback function --- packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol index 4c80ab9ed2..62aaea96d4 100644 --- a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol +++ b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol @@ -40,9 +40,6 @@ contract AirnodeRrpV0DryRun /// the fulfillment. All of its keys will map to zero values. mapping(bytes32 => bytes32) private requestIdToFulfillmentParameters; - // solhint-disable-next-line payable-fallback - fallback() external { } - /// @notice Used by Airnode to estimate the gas amount needed to fulfill /// the request (excluding the external call). Do not call this function, /// as it will have no practical effect. From 43cb3a2c223608ceb1fe672d8377615c8983f267 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Fri, 14 Apr 2023 10:34:24 +0300 Subject: [PATCH 6/7] Improve comment wording --- .../airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol index 62aaea96d4..9f9f5b0df3 100644 --- a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol +++ b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol @@ -63,8 +63,8 @@ contract AirnodeRrpV0DryRun ) external returns (bool callSuccess, bytes memory callData) { // The line below is kept the same, except that the condition is // reversed to ensure that it never reverts. All - // `requestIdToFulfillmentParameters` values are zero and virtually any - // `keccak256()` output will not be equal to that. + // `requestIdToFulfillmentParameters` values are zero and virtually no + // `keccak256()` output will be equal to that. require( keccak256( abi.encodePacked( From 8698a82312a8c8f388e9ada474ea02acf775a9c8 Mon Sep 17 00:00:00 2001 From: bbenligiray Date: Fri, 14 Apr 2023 10:34:55 +0300 Subject: [PATCH 7/7] Use the Airnode address as the dummy call target --- .../contracts/rrp/AirnodeRrpV0DryRun.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol index 9f9f5b0df3..a9ad897479 100644 --- a/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol +++ b/packages/airnode-protocol/contracts/rrp/AirnodeRrpV0DryRun.sol @@ -33,9 +33,6 @@ contract AirnodeRrpV0DryRun bytes data ); - // It is virtually impossible for a contract to be deployed at this address - address private constant ADDRESS_WITH_NO_BYTECODE = address(uint160(uint256(keccak256("Address with no bytecode")))); - /// @dev This mapping is kept as it is in AirnodeRrpV0 to closely simulate /// the fulfillment. All of its keys will map to zero values. mapping(bytes32 => bytes32) private requestIdToFulfillmentParameters; @@ -89,8 +86,11 @@ contract AirnodeRrpV0DryRun // function will be behind an `onlyAirnodeRrp` modifier and will reject // the calls from AirnodeRrpV0DryRun. // Instead, we call an address that we know to not contain any - // bytecode, which means this call will not revert or spend extra gas. - (callSuccess, callData) = ADDRESS_WITH_NO_BYTECODE.call( // solhint-disable-line avoid-low-level-calls + // bytecode, which will result in the call to not revert or spend extra + // gas. Since we have already confirmed that `airnode` has signed a + // hash, it is guaranteed to be an EOA and we can use it as a dummy + // call target. + (callSuccess, callData) = airnode.call( // solhint-disable-line avoid-low-level-calls abi.encodeWithSelector(fulfillFunctionId, requestId, data) ); // If the external call above does not succeed, the `eth_estimateGas`