Omniscia Tangible Audit

GelatoVRFConsumerBase Code Style Findings

GelatoVRFConsumerBase Code Style Findings

GVR-01C: Inefficient Data Structure

Description:

The GelatoVRFConsumerBase makes use of a bool array in an append-only fashion which is significantly inefficient.

Example:

src/abstract/GelatoVRFConsumerBase.sol
36/// @notice Requests randomness from the Gelato VRF.
37/// @dev The extraData parameter allows for additional data to be passed to
38/// the VRF, which is then forwarded to the callback. This is useful for
39/// request tracking purposes if requestId is not enough.
40/// @param extraData Additional data for the randomness request.
41/// @return requestId The ID for the randomness request.
42function _requestRandomness(
43    bytes memory extraData
44) internal returns (uint256 requestId) {
45    requestId = uint256(requestPending.length);
46    requestPending.push();
47    requestPending[requestId] = true;
48

49    bytes memory data = abi.encode(requestId, extraData);
50    uint256 round = _round();
51

52    bytes memory dataWithRound = abi.encode(round, data);
53    bytes32 requestHash = keccak256(dataWithRound);
54

55    requestedHash[requestId] = requestHash;
56    rawRequestedHash[requestId] = dataWithRound;
57

58    emit RequestedRandomness(round, data);
59}

Recommendation:

We advise the system to utilize a mapping instead alongside an length member that represents the maximum ID that has been assigned to the mapping. As an additional optimization, the underlying data type can be changed to a uint256 that is set from 0 to 1 optimizing the code further as the EVM is better suited for dealing with 32-byte data types.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The Tangible team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

GVR-02C: Redundant Casting Operation

Description:

The referenced statement will cast the requestPending.length member to an uint256 while it is already represented by the said type.

Example:

src/abstract/GelatoVRFConsumerBase.sol
45requestId = uint256(requestPending.length);

Recommendation:

We advise the casting operation to be omitted, optimizing the legibility of the code.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The Tangible team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

GVR-03C: Suboptimal Segregation of Data

Description:

The requestedHash and rawRequestedHash variables represent mapping declarations that accept the same key and are utilized together in all instances of the code.

Example:

src/abstract/GelatoVRFConsumerBase.sol
36/// @notice Requests randomness from the Gelato VRF.
37/// @dev The extraData parameter allows for additional data to be passed to
38/// the VRF, which is then forwarded to the callback. This is useful for
39/// request tracking purposes if requestId is not enough.
40/// @param extraData Additional data for the randomness request.
41/// @return requestId The ID for the randomness request.
42function _requestRandomness(
43    bytes memory extraData
44) internal returns (uint256 requestId) {
45    requestId = uint256(requestPending.length);
46    requestPending.push();
47    requestPending[requestId] = true;
48

49    bytes memory data = abi.encode(requestId, extraData);
50    uint256 round = _round();
51

52    bytes memory dataWithRound = abi.encode(round, data);
53    bytes32 requestHash = keccak256(dataWithRound);
54

55    requestedHash[requestId] = requestHash;
56    rawRequestedHash[requestId] = dataWithRound;
57

58    emit RequestedRandomness(round, data);
59}
60

61/// @notice Testnet Method to make mocked vrf callbacks easier.
62/// @param randomness The random number generated by Gelato VRF.
63/// @param requestId The ID for the randomness request that's being fulfilled.
64function fulfillRandomnessTestnet(
65    uint256 randomness,
66    uint256 requestId
67) external {
68    require(block.chainid == testnetChainId, "Method only accessible on testnet");
69

70    bytes storage dataWithRound = rawRequestedHash[requestId];
71

72    (, bytes memory data) = abi.decode(dataWithRound, (uint256, bytes));
73    (uint256 requestId, bytes memory extraData) = abi.decode(
74        data,
75        (uint256, bytes)
76    );
77

78    bytes32 requestHash = keccak256(dataWithRound);
79    bool isValidRequestHash = requestHash == requestedHash[requestId];
80

81    require(requestPending[requestId], "request fulfilled or missing");
82

83    if (isValidRequestHash) {
84        randomness = uint(
85            keccak256(
86                abi.encode(
87                    randomness,
88                    address(this),
89                    block.chainid,
90                    requestId
91                )
92            )
93        );
94

95        _fulfillRandomness(randomness, requestId, extraData);
96        requestPending[requestId] = false;
97    }
98}

Recommendation:

We advise a single mapping to be declared that points to a struct with a bytes32 and bytes entry, optimizing the overall gas cost of the contract significantly.

Alleviation (106fc61dcd38fe29a81d1984ccde9171f5f231af):

The Tangible team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

Prev

BasketsVrfConsumer.sol (BVC-C)

Next

VRFConsumerBaseV2Upgradeable.sol (VRF-C)

On this page