-
Notifications
You must be signed in to change notification settings - Fork 72
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.
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
Add AirnodeRrpV0DryRun #1737
Add AirnodeRrpV0DryRun #1737
Changes from all commits
87a156e
b61a581
14f7d00
134b6ff
2121175
43cb3a2
8698a82
8f15c41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@api3/airnode-protocol': minor | ||
--- | ||
|
||
Add AirnodeRrpV0DryRun.sol |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
// 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 | ||
); | ||
|
||
/// @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; | ||
|
||
/// @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 no | ||
// `keccak256()` output will 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 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` | ||
// 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); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
Comment on lines
+110
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏻 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's not guaranteed if it would be less or more because
So it would depend on how heavy AirnodeRrpV0's fulfill()'s external call will be, which makes this not a very good test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree. I'm just trying to see if we can find a more future-proof way of testing the estimageGas() response by using a relative comparison instead of hardcoded numbers that may become obsolete on different chains, etc. But I guess we can do that once (and if) this becomes an issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was mainly testing that it returns some kind of a number without erroring here. I don't think the value is very important as long as it's something completely wrong like |
||
}); | ||
}); | ||
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'); | ||
}); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you saying it will always return the gas estimate for when the airnode emits
FailedRequest
becase of the wayestimateGas
short-circuits ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This was originally picked up by e2e tests for #1589 was failing.