Use of return value from assignment hampers readability

[code423n4]

Handle

TomFrenchBlockchain

Vulnerability details

Impact

Reduced readability

Proof of Concept

In a number of placed we seem to be inlining an assignment with the usage of that variable:

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L40

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L70

https://github.com/XDeFi-tech/xdefi-distribution/blob/3856a42df295183b40c6eee89307308f196612fe/contracts/XDEFIDistribution.sol#L83

This is quite atypical in my experience and reduces readability: lines which contain require statements and event emission now modify contract storage.

Recommended Mitigation Steps

Consider whether any small benefits to gas/compactness are worth the reduced clarity.

[deluca-mike]

Agreed. Will go with the option to assign return variables in order to keep the function prototype pattern/style consistent (i.e. return variables all have names).

[deluca-mike]

You can see in the release candidate contract that these are all fixed, but specifically:

• emergencyUnlock (did not exist before)
• lock
• lockWithPermit
• withdrawableOf
• getScore (formerly getPoints)
• scoreOf (formerly pointsOf)
• tokenURI
• _generateNewTokenId
• _getScore (formerly _getPoints)
• _getScoreFromTokenId (formerly _getPointsFromTokenId)
• _lock (former _lock became _createLockedPosition, so this function is new)
• _updateDistributableXDEFI (formerly _updateXDEFIBalance)
• _withdrawableGiven