MetaSoccerScouting Code Style Findings

MSS-01C: Illegible Struct Initialization

Type	Severity	Location
Code Style		MetaSoccerScouting.sol:L326

Description:

The Scouting struct initialization in the linked statement is slightly illegible as it uses index-based assignments.

Example:

Copy
326Scouting memory scouting = Scouting(false, false, _level, _covering, _owner, block.timestamp, _scoutId, playerIds, _scoutingPeriod);

Recommendation:

We advise the new key-value format to be utilized here instead increasing the legibility of the statement significantly.

Alleviation:

The key-value format is now utilized in the codebase as advised.

MSS-02C: Inefficient Loop Limit Evaluations

Type	Severity	Location
Gas Optimization		MetaSoccerScouting.sol:L131, L242, L253, L264

Description:

The linked loop limit evaluations are performed dynamically increasing the gas cost of the functions they are located in significantly.

Example:

Copy
120/**
121 * @dev Scout NFT attributes will evolve off-chain, but scouting level will depend on them. 
122 * The decided approach to solve this issue is that the backend will "authorize" the scouting
123 * by signing an EIP712 compliant message including the Scout ID, level and covering.
124 * The signer account requires the SIGN_ATTRIBUTES_ROLE role.
125 */
126function sendToScouting(uint256 _scoutId, uint8 _level, string calldata _covering, uint256 _scoutingPeriod, uint256 _timestamp, bytes calldata _signature) external nonReentrant whenNotPaused {
127    address owner = ERC721(scouts).ownerOf(_scoutId);
128    require(_msgSender() == owner, "Not Scout Owner");
129    _verifyScoutingRequestSignature(_scoutId, _level, _covering, _scoutingPeriod, _timestamp, _signature);
130
131    for (uint256 i = 0; i < paymentTokens.length; i++) {
132        uint256 price = priceByLevel[paymentTokens[i]][_level];
133        if (price > 0) {
134            IERC20(paymentTokens[i]).safeTransferFrom(owner, beneficiary, price);
135        }
136    }
137
138    _startScouting(_scoutId, _level, _covering, _scoutingPeriod, owner);
139}

Recommendation:

We advise them to be stored to a local variable that is consequently utilized for the for loop declaration optimizing their respective gas costs.

Alleviation:

The optimization advised was only applied for the depicted instance and was not applied in all linked instances thereby partially alleviating this exhibit.

MSS-03C: Inefficient Seed Offset Storing

Type	Severity	Location
Gas Optimization		MetaSoccerScouting.sol:L184, L192, L195

Description:

The seedIndex variable is meant to retain the offset utilized to pass in the correct seeds entry to each setEntropy call of the generated players.

Example:

Copy
184uint256 seedIndex = 0;
185scouting.claimed = true;
186uint256[] memory playerIds = new uint256[](playersToMint);
187for (uint8 i = 0; i < playersToMint; i++) {
188    uint256 playerId = players.mintPlayer(owner, 0, _scoutingId);
189    playerIds[i] = playerId;
190
191    for(uint8 j = 0; j < requiredPlayerSeeds; j++) {
192        players.setEntropy(playerId, j, seeds[seedIndex + j]);
193    }
194
195    seedIndex += requiredPlayerSeeds;
196}

Recommendation:

We advise the offset to be calculated dynamically instead as a result of i * requiredPlayerSeeds, reducing the gas cost and complexity of the function.

Alleviation:

The seedIndex value is now properly calculated on each iteration optimizing the function's gas cost.

MSS-04C: Inexistent Visibility Specifier

Type	Severity	Location
Code Style		MetaSoccerScouting.sol:L51

Description:

The linked variable has no visibility specifier explicitly set.

Example:

Copy
51MetaSoccerPlayers players;

Recommendation:

We advise one to be set so to avoid potential compilation discrepancies in the future as the current behaviour is for the compiler to assign one automatically which may deviate between pragma versions.

Alleviation:

The public visibility specifier was introduced to the linked variable.

MSS-05C: Variable Mutability Specifiers

Type	Severity	Location
Gas Optimization		MetaSoccerScouting.sol:L42, L51

Description:

The linked variables are assigned to only once during the contract's constructor.

Example:

Copy
72constructor(
73    address _scouts,
74    MetaSoccerPlayers _players,
75    address _entropyManager,
76    string memory _nftName,
77    string memory _nftSymbol
78) ERC721(_nftName, _nftSymbol) EIP712("MetaSoccer", "1") {
79    require(_scouts != address(0), "Wrong Scouts address");
80    scouts = _scouts;
81    players = _players;
82    entropyManager = _entropyManager;
83    _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
84}

Recommendation:

We advise them to be set as immutable greatly optimizing the codebase.

Alleviation:

The immutable specifier was only introduced for the players value and not the entropyManager thereby partially alleviating this exhibit.