FragmentNFT Static Analysis Findings

FNF-01S: Data Location Optimizations

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L277, L278

Description:

The linked input arguments are set as memory in external function(s).

Example:

Copy
276function proposeMany(
277  address[] memory owners,
278  bytes32[] memory tags_,
279  bytes calldata signature
280) external onlyDatasetNFT {

Recommendation:

We advise them to be set as calldata optimizing their read-access gas cost.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The FragmentNFT::proposeMany function's eligible input arguments have been properly set to calldata, optimizing their read-access gas cost.

FNF-02S: Inexistent Validation of Proposal Recipients

Type	Severity	Location
Input Sanitization		FragmentNFT.sol:L253, L277

Description:

The FragmentNFT::propose and FragmentNFT::proposeMany functions do not validate their perspective address arguments (to / owners) as being non-zero, potentially leading to proposals that can neither be accepted or rejected.

Impact:

The current FragmentNFT system permits unfulfillable proposals to be created and the relevant DatasetNFT functions (DatasetNFT::proposeFragment / DatasetNFT::proposeManyFragments) do not perform any validation to prevent this.

Example:

Copy
309/**
310 * @notice Accepts a specific proposed contribution by minting the respective pending Fragment NFT to contributor
311 * @dev Only callable by VerifierManager contract.
312 * Emits a {FragmentAccepted} event.
313 * @param id The ID of the pending Fragment NFT associated with the proposed contribution to be accepted
314 */
315function accept(uint256 id) external onlyVerifierManager {
316  address to = pendingFragmentOwners[id];
317  if (_exists(id) || to == address(0)) revert NOT_PENDING_FRAGMENT(id);
318  delete pendingFragmentOwners[id];
319  _safeMint(to, id);
320  emit FragmentAccepted(id);
321}
322
323/**
324 * @notice Rejects a specific proposed contribution by removing the associated pending Fragment NFT
325 * @dev Only callable by VerifierManager contract.
326 * Emits a {FragmentRejected} event.
327 * @param id The ID of the pending Fragment NFT associated with the proposed contribution to be rejected
328 */
329function reject(uint256 id) external onlyVerifierManager {
330  address to = pendingFragmentOwners[id];
331  if (_exists(id) || to == address(0)) revert NOT_PENDING_FRAGMENT(id);
332  delete pendingFragmentOwners[id];
333  delete tags[id];
334  emit FragmentRejected(id);
335}

Recommendation:

We advise the code to properly validate the address arguments as being non-zero, preventing unfulfillable proposals from being created that would need to be manually removed via the FragmentNFT::remove function available to the data-set owner.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The code of both the FragmentNFT::propose and the FragmentNFT::proposeMany functions has been updated to ensure that the to address is not zero in the former case and to skip entries where the to address (owners[i]) would be zero in the latter case, alleviating this exhibit in full.