[M-1] Silent Return on Error in fulfillMintRequest Function Causes Loss of User Feedback
Description
The fulfillMintRequest function is designed to process oracle responses for weather NFT minting requests, and when successful, mint a new NFT to the user.
However, due to a logic error, the function silently returns without any feedback when an oracle error occurs, leaving users confused and without their NFTs.
function fulfillMintRequest(bytes32 requestId) external {
bytes memory response = s_funcReqIdToMintFunctionReqResponse[requestId].response;
bytes memory err = s_funcReqIdToMintFunctionReqResponse[requestId].err;
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
@> if (response.length == 0 || err.length > 0) {
@> return;
@> }
}
Risk
Likelihood: High
-
Oracle errors are common in Web3 applications due to network issues, API failures, or data availability problems
-
Any error response from the oracle will trigger this condition
-
The function doesn't revert or emit events in error cases, making it hard to detect
Impact: Medium
-
Users experience transactions that theoretically succeed (don't revert) but don't actually mint any NFTs
-
No feedback mechanism exists to inform users about errors or failed requests
-
User funds (gas costs and potentially LINK deposits) are spent without clear outcome
-
Request data remains in storage indefinitely, potentially causing bloat
-
Users have no way to retry or troubleshoot failed requests
Proof of Concept
Run the test bellow in your testing suite:
function test_silentReturn_whenErrorResponseExists() public {
string memory pincode = "125001";
string memory isoCode = "IN";
bool registerKeeper = true;
uint256 heartbeat = 12 hours;
uint256 initLinkDeposit = 5e18;
uint256 initialTokenCount = weatherNft.s_tokenCounter();
console.log("Initial token count: ", initialTokenCount);
console.log("Initial price: ", weatherNft.s_currentMintPrice());
vm.startPrank(user);
linkToken.approve(address(weatherNft), initLinkDeposit);
vm.recordLogs();
weatherNft.requestMintWeatherNFT{
value: weatherNft.s_currentMintPrice()
}(pincode, isoCode, registerKeeper, heartbeat, initLinkDeposit);
vm.stopPrank();
Vm.Log[] memory logs = vm.getRecordedLogs();
bytes32 reqId;
for (uint256 i; i < logs.length; i++) {
if (
logs[i].topics[0] ==
keccak256(
"WeatherNFTMintRequestSent(address,string,string,bytes32)"
)
) {
(, , , reqId) = abi.decode(
logs[i].data,
(address, string, string, bytes32)
);
break;
}
}
assert(reqId != bytes32(0));
vm.prank(functionsRouter);
weatherNft.handleOracleFulfillment(reqId, "", "Error fetching data");
vm.prank(user);
weatherNft.fulfillMintRequest(reqId);
assertEq(weatherNft.s_tokenCounter(), initialTokenCount);
assertEq(weatherNft.balanceOf(user), 0);
(uint256 reqHeartbeat, address reqUser, , , , ) = weatherNft
.s_funcReqIdToUserMintReq(reqId);
assertEq(reqUser, user);
assertEq(reqHeartbeat, heartbeat);
}
As shown and explained above, we have no way of telling if the transaction truly failed, instead it will just return nothing and the user will not get any NFT.
Recommended Mitigation
Consider adding a few new custom errors to check for potential issues, and clear storage so that it doesn't bloat and increase gas costs.
function fulfillMintRequest(bytes32 requestId) external {
//...function logic
require(response.length > 0 || err.length > 0, WeatherNft__Unauthorized());
- if (response.length == 0 || err.length > 0) {
- return;
- }
+ // Revert with proper error message when there's a problem with the oracle response
+ if (err.length > 0) {
+ revert WeatherNft__OracleError(requestId, err);
+ }
+
+ // Ensure there's a valid response
+ require(response.length > 0, WeatherNft__EmptyResponse(requestId));
// Rest of function...
+ // Clean up storage
+ delete s_funcReqIdToMintFunctionReqResponse[requestId];
+ delete s_funcReqIdToUserMintReq[requestId];
}
// Add new error types
+ error WeatherNft__OracleError(bytes32 requestId, bytes errorMessage);
+ error WeatherNft__EmptyResponse(bytes32 requestId);
