diff --git a/book/docs/consumers/on-chain.md b/book/docs/consumers/on-chain.md index 1687c25..8c8aa90 100644 --- a/book/docs/consumers/on-chain.md +++ b/book/docs/consumers/on-chain.md @@ -47,8 +47,9 @@ function fulfill( } ``` -(`requestHash` is `keccak256(endpointId, timestamp, data)` and is AirnodeVerifier's replay key — each unique payload can -only be fulfilled once, globally.) +(`requestHash` is `keccak256(endpointId, timestamp, data)`. Replay protection is scoped by signer, request hash, +callback address, and selector. The public `fulfilled(airnode, requestHash)` getter indicates that the payload has been +delivered at least once; `fulfilledDelivery(deliveryHash)` tracks the precise tuple.) ### Security checklist @@ -76,7 +77,8 @@ The single most likely place a consumer loses money is forgetting one of these. shape the requester picked. If a check fails, **revert** (or ignore) — and note that a revert inside `fulfill` does not revert AirnodeVerifier's -`verifyAndFulfill`: the request is still marked fulfilled (anti-griefing), your state just stays put. +`verifyAndFulfill`: that precise signer/payload/callback/selector delivery remains recorded, while another callback or +selector can still receive the same signed payload. ### When to use diff --git a/book/docs/contracts/verifier.md b/book/docs/contracts/verifier.md index dae8b7c..4b66f3c 100644 --- a/book/docs/contracts/verifier.md +++ b/book/docs/contracts/verifier.md @@ -47,9 +47,9 @@ function fulfill( ## Replay protection -The `requestHash` (the `messageHash` from the signature) serves as the replay key. Each unique combination of -`(endpointId, timestamp, data)` can only be fulfilled once. The `fulfilled` mapping is public -- anyone can check -whether a particular hash has been submitted. +The `requestHash` is the `messageHash` from the signature. Replay protection is scoped by signer, so each unique +`(airnode, endpointId, timestamp, data)` combination can only be fulfilled once while independent airnodes can attest +the same payload. The nested `fulfilled(airnode, requestHash)` mapping is public. ## Trust model diff --git a/contracts/README.md b/contracts/README.md index b95cebf..627096d 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -94,9 +94,9 @@ function fulfill( ### Replay protection -The `requestHash` (which is the `messageHash` from the signature) serves as the replay key. Each unique combination of -`(endpointId, timestamp, data)` can only be fulfilled once. The `fulfilled` mapping is public — anyone can check whether -a particular hash has been submitted. +The `requestHash` is the `messageHash` from the signature. Replay protection is scoped by signer: each unique +`(airnode, endpointId, timestamp, data)` combination can only be fulfilled once. The nested `fulfilled` mapping is +public, so callers can query `fulfilled(airnode, requestHash)`. ### Trust model diff --git a/contracts/src/AirnodeVerifier.sol b/contracts/src/AirnodeVerifier.sol index c4d1840..1e279ae 100644 --- a/contracts/src/AirnodeVerifier.sol +++ b/contracts/src/AirnodeVerifier.sol @@ -23,12 +23,12 @@ pragma solidity ^0.8.24; /// - The contract only verifies the signature. It does NOT check whether the /// airnode is "legitimate" — that is the callback contract's responsibility. /// The callback contract should maintain its own trust set of airnode addresses. -/// - Replay protection: each (endpointId, timestamp, data) combination can only -/// be fulfilled once. +/// - Replay protection: each (airnode, endpointId, timestamp, data, callback, +/// selector) combination can only be fulfilled once. Independent airnodes +/// and consumers can deliver the same payload without blocking each other. /// - The callback receives (requestHash, airnode, endpointId, timestamp, data) /// so it has all the context it needs to validate and process the data. -/// - If the callback reverts, the fulfillment is still recorded. This prevents -/// griefing where a callback intentionally reverts to block fulfillment. +/// - If the callback reverts, that precise delivery tuple is still recorded. contract AirnodeVerifier { // =========================================================================== // Events @@ -44,9 +44,11 @@ contract AirnodeVerifier { // =========================================================================== // Storage // =========================================================================== - /// @notice Tracks fulfilled requests to prevent replay. The key is the hash of the - /// signed message, which is unique per (endpointId, timestamp, data) combination. - mapping(bytes32 => bool) public fulfilled; + /// @notice Indicates whether a signer/payload pair has been delivered at least once. + mapping(address => mapping(bytes32 => bool)) public fulfilled; + + /// @notice Tracks the precise signer/payload/callback/selector delivery tuple. + mapping(bytes32 => bool) public fulfilledDelivery; // =========================================================================== // External functions @@ -79,9 +81,13 @@ contract AirnodeVerifier { address recovered = _recover(ethSignedHash, signature); require(recovered == airnode, 'Signature mismatch'); - // Prevent replay — each unique message can only be fulfilled once - require(!fulfilled[messageHash], 'Already fulfilled'); - fulfilled[messageHash] = true; + // Replay protection is scoped to the signer and callback target. Two + // independent consumers may legitimately deliver the same attestation, and + // a caller cannot burn it by supplying an unrelated callback or selector. + bytes32 deliveryHash = keccak256(abi.encode(airnode, messageHash, callbackAddress, callbackSelector)); + require(!fulfilledDelivery[deliveryHash], 'Already fulfilled'); + fulfilledDelivery[deliveryHash] = true; + fulfilled[airnode][messageHash] = true; // Emit before external interaction (checks-effects-interactions pattern) emit Fulfilled(messageHash, airnode, endpointId, timestamp, callbackAddress); diff --git a/contracts/test/AirnodePriceConsumer.t.sol b/contracts/test/AirnodePriceConsumer.t.sol index 900696a..52da67d 100644 --- a/contracts/test/AirnodePriceConsumer.t.sol +++ b/contracts/test/AirnodePriceConsumer.t.sol @@ -153,7 +153,7 @@ contract AirnodePriceConsumerTest is Test { verifier.verifyAndFulfill(airnode, wrongEndpoint, TIMESTAMP, data, sig, address(consumer), SELECTOR); - assertTrue(verifier.fulfilled(keccak256(abi.encodePacked(wrongEndpoint, TIMESTAMP, data)))); + assertTrue(verifier.fulfilled(airnode, keccak256(abi.encodePacked(wrongEndpoint, TIMESTAMP, data)))); assertEq(consumer.latestTimestamp(), 0); // consumer rejected it (WrongEndpoint) } } diff --git a/contracts/test/AirnodeVerifier.symbolic.t.sol b/contracts/test/AirnodeVerifier.symbolic.t.sol index 9b6b06c..2fddff0 100644 --- a/contracts/test/AirnodeVerifier.symbolic.t.sol +++ b/contracts/test/AirnodeVerifier.symbolic.t.sol @@ -30,7 +30,7 @@ contract AirnodeVerifierSymbolicTest is Test { bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); // Pre-condition: not already fulfilled - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); bytes32 messageHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); bytes32 ethSignedHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', messageHash)); @@ -47,7 +47,7 @@ contract AirnodeVerifierSymbolicTest is Test { MockCallback.fulfill.selector ); - assert(verifier.fulfilled(requestHash)); + assert(verifier.fulfilled(airnodeAddress, requestHash)); } /// @notice Replay always reverts — for any hash, fulfilling twice fails @@ -62,7 +62,7 @@ contract AirnodeVerifierSymbolicTest is Test { bytes memory sig = abi.encodePacked(r, s, v); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); // First call succeeds verifier.verifyAndFulfill( diff --git a/contracts/test/AirnodeVerifier.t.sol b/contracts/test/AirnodeVerifier.t.sol index 9820edd..8b581ba 100644 --- a/contracts/test/AirnodeVerifier.t.sol +++ b/contracts/test/AirnodeVerifier.t.sol @@ -11,6 +11,7 @@ contract AirnodeVerifierTest is Test { RevertingCallback revertingCallback; uint256 constant AIRNODE_KEY = 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80; + uint256 constant SECOND_AIRNODE_KEY = 0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d; address airnodeAddress; bytes32 constant ENDPOINT_ID = bytes32(uint256(1)); @@ -33,9 +34,18 @@ contract AirnodeVerifierTest is Test { // =========================================================================== function _sign(bytes32 endpointId, uint256 timestamp, bytes memory data) internal pure returns (bytes memory) { + return _signWithKey(AIRNODE_KEY, endpointId, timestamp, data); + } + + function _signWithKey( + uint256 key, + bytes32 endpointId, + uint256 timestamp, + bytes memory data + ) internal pure returns (bytes memory) { bytes32 messageHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); bytes32 ethSignedHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', messageHash)); - (uint8 v, bytes32 r, bytes32 s) = vm.sign(AIRNODE_KEY, ethSignedHash); + (uint8 v, bytes32 r, bytes32 s) = vm.sign(key, ethSignedHash); return abi.encodePacked(r, s, v); } @@ -59,11 +69,11 @@ contract AirnodeVerifierTest is Test { bytes memory sig = _sign(ENDPOINT_ID, TIMESTAMP, DATA); bytes32 requestHash = keccak256(abi.encodePacked(ENDPOINT_ID, TIMESTAMP, DATA)); - assertFalse(verifier.fulfilled(requestHash)); + assertFalse(verifier.fulfilled(airnodeAddress, requestHash)); verifier.verifyAndFulfill(airnodeAddress, ENDPOINT_ID, TIMESTAMP, DATA, sig, address(callback), CALLBACK_SELECTOR); - assertTrue(verifier.fulfilled(requestHash)); + assertTrue(verifier.fulfilled(airnodeAddress, requestHash)); } function test_reverts_on_replay() public { @@ -75,6 +85,36 @@ contract AirnodeVerifierTest is Test { verifier.verifyAndFulfill(airnodeAddress, ENDPOINT_ID, TIMESTAMP, DATA, sig, address(callback), CALLBACK_SELECTOR); } + function test_allows_independent_airnodes_to_submit_the_same_payload() public { + address secondAirnode = vm.addr(SECOND_AIRNODE_KEY); + bytes memory firstSignature = _sign(ENDPOINT_ID, TIMESTAMP, DATA); + bytes memory secondSignature = _signWithKey(SECOND_AIRNODE_KEY, ENDPOINT_ID, TIMESTAMP, DATA); + + verifier.verifyAndFulfill( + airnodeAddress, + ENDPOINT_ID, + TIMESTAMP, + DATA, + firstSignature, + address(callback), + CALLBACK_SELECTOR + ); + verifier.verifyAndFulfill( + secondAirnode, + ENDPOINT_ID, + TIMESTAMP, + DATA, + secondSignature, + address(callback), + CALLBACK_SELECTOR + ); + + bytes32 requestHash = keccak256(abi.encodePacked(ENDPOINT_ID, TIMESTAMP, DATA)); + assertTrue(verifier.fulfilled(airnodeAddress, requestHash)); + assertTrue(verifier.fulfilled(secondAirnode, requestHash)); + assertEq(callback.callCount(), 2); + } + function test_reverts_on_wrong_airnode() public { bytes memory sig = _sign(ENDPOINT_ID, TIMESTAMP, DATA); address wrongAirnode = address(0xdead); @@ -103,7 +143,7 @@ contract AirnodeVerifierTest is Test { bytes memory sig = _sign(ENDPOINT_ID, TIMESTAMP, DATA); bytes32 requestHash = keccak256(abi.encodePacked(ENDPOINT_ID, TIMESTAMP, DATA)); - // Should not revert — the callback reverts but the fulfillment is recorded + // Should not revert — the callback reverts but that precise delivery is recorded. verifier.verifyAndFulfill( airnodeAddress, ENDPOINT_ID, @@ -114,7 +154,34 @@ contract AirnodeVerifierTest is Test { RevertingCallback.fulfill.selector ); - assertTrue(verifier.fulfilled(requestHash)); + assertTrue(verifier.fulfilled(airnodeAddress, requestHash)); + } + + function test_wrong_callback_cannot_burn_intended_delivery() public { + bytes memory sig = _sign(ENDPOINT_ID, TIMESTAMP, DATA); + + verifier.verifyAndFulfill( + airnodeAddress, + ENDPOINT_ID, + TIMESTAMP, + DATA, + sig, + address(revertingCallback), + RevertingCallback.fulfill.selector + ); + + verifier.verifyAndFulfill(airnodeAddress, ENDPOINT_ID, TIMESTAMP, DATA, sig, address(callback), CALLBACK_SELECTOR); + + assertEq(callback.callCount(), 1); + } + + function test_wrong_selector_cannot_burn_intended_delivery() public { + bytes memory sig = _sign(ENDPOINT_ID, TIMESTAMP, DATA); + + verifier.verifyAndFulfill(airnodeAddress, ENDPOINT_ID, TIMESTAMP, DATA, sig, address(callback), bytes4(0xdeadbeef)); + verifier.verifyAndFulfill(airnodeAddress, ENDPOINT_ID, TIMESTAMP, DATA, sig, address(callback), CALLBACK_SELECTOR); + + assertEq(callback.callCount(), 1); } function test_different_data_produces_different_request_hash() public { diff --git a/contracts/test/ConfidentialPriceFeed.invariant.t.sol b/contracts/test/ConfidentialPriceFeed.invariant.t.sol index 05ec5c7..282381c 100644 --- a/contracts/test/ConfidentialPriceFeed.invariant.t.sol +++ b/contracts/test/ConfidentialPriceFeed.invariant.t.sol @@ -56,7 +56,7 @@ contract ConfidentialFeedHandler is Test { bytes32 messageHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); // Skip if already fulfilled in verifier (would revert) - if (verifier.fulfilled(messageHash)) return; + if (verifier.fulfilled(airnodeAddress, messageHash)) return; bytes32 ethSignedHash = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', messageHash)); (uint8 v, bytes32 r, bytes32 s) = vm.sign(AIRNODE_KEY, ethSignedHash); diff --git a/contracts/test/ConfidentialPriceFeed.symbolic.t.sol b/contracts/test/ConfidentialPriceFeed.symbolic.t.sol index ed4e2c5..8b0f5e8 100644 --- a/contracts/test/ConfidentialPriceFeed.symbolic.t.sol +++ b/contracts/test/ConfidentialPriceFeed.symbolic.t.sol @@ -68,7 +68,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes memory data = abi.encode(handleRef, inputProof); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); _fulfillViaVerifier(endpointId, timestamp, data); @@ -97,7 +97,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes memory data = abi.encode(handleRef, inputProof); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); _fulfillViaVerifier(endpointId, timestamp, data); @@ -128,7 +128,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes memory data = abi.encode(handleRef, inputProof); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); _fulfillViaVerifier(endpointId, timestamp, data); @@ -160,7 +160,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes memory data = abi.encode(handleRef, inputProof); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, requestHash)); _fulfillViaVerifier(endpointId, timestamp, data); @@ -188,7 +188,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes32 freshRef = bytes32(uint256(0xfeed)); bytes memory freshData = abi.encode(freshRef, hex'aa'); bytes32 freshHash = keccak256(abi.encodePacked(endpointId, freshTs, freshData)); - vm.assume(!verifier.fulfilled(freshHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, freshHash)); _fulfillViaVerifier(endpointId, freshTs, freshData); @@ -199,7 +199,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes32 staleRef = bytes32(uint256(0xdead)); bytes memory staleData = abi.encode(staleRef, hex'bb'); bytes32 staleHash = keccak256(abi.encodePacked(endpointId, staleTs, staleData)); - vm.assume(!verifier.fulfilled(staleHash)); + vm.assume(!verifier.fulfilled(airnodeAddress, staleHash)); _fulfillViaVerifier(endpointId, staleTs, staleData); @@ -232,7 +232,7 @@ contract ConfidentialPriceFeedSymbolicTest is Test { bytes memory sig = abi.encodePacked(r, s, v); bytes32 requestHash = keccak256(abi.encodePacked(endpointId, timestamp, data)); - vm.assume(!verifier.fulfilled(requestHash)); + vm.assume(!verifier.fulfilled(untrustedAirnode, requestHash)); // This will revert at the verifier level ("Signature mismatch") since we // pass airnodeAddress but signed with a different key. If we passed