AsRescuable Code Style Findings

ARE-01C: Improper Declarations of Abstract Functions

Type	Severity	Location
Standard Conformity		AsRescuable.sol:L65, L98

Description:

The AsRescuable::requestRescue and AsRescuable::rescue functions are meant to be virtual and implemented by derivative implementations, however, an empty declaration is present in both that would permit each to be invoked.

Example:

Copy
64// to be overriden with the proper access control by inheriting contracts
65function requestRescue(address _token) external virtual {}

Recommendation:

We advise the functions to be declared without a code block ({}) to ensure they are overridden by derivative implementations.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

Both functions are now fully implemented by the AsRescuable implementation directly, rendering this exhibit no longer applicable.

ARE-02C: Inefficient Erasure of Request

Type	Severity	Location
Gas Optimization		AsRescuable.sol:L83-L84, L92-L93

Description:

The AsRescuable::_rescue function will erase the rescueRequests entry after the token has been transferred but will mark its timestamp as 0 before to prevent re-entrancies.

Example:

Copy
67/**
68 * @dev Internal function to rescue tokens or native tokens (ETH) from the contract.
69 * @param _token The address of the token to be rescued. Use address(1) for native tokens (ETH).
70 * @notice This function can only be called by the receiver specified in the rescue request.
71 * @notice The rescue request must be initiated before the rescue timelock expires.
72 * @notice The rescue request remains valid until the rescue validity period expires.
73 * @notice If the rescue request is valid, the specified amount of tokens will be transferred to the receiver.
74 * @notice If the rescue request is not valid, a new rescue request will be set with the caller as the receiver.
75 * @notice Emits a Rescue event when the rescue is successful.
76 * @notice Emits a Rescue event when a new rescue request is set.
77 */
78function _rescue(address _token) internal {
79    RescueRequest storage req = rescueRequests[_token];
80    // check if rescue is pending
81    require(_isRescueUnlocked(req));
82
83    // reset timestamp to prevent reentrancy
84    rescueRequests[_token].timestamp = 0;
85
86    // send to receiver
87    if (_token == address(1)) {
88        payable(req.receiver).transfer(address(this).balance);
89    } else {
90        IERC20Metadata(_token).safeTransfer(req.receiver, IERC20Metadata(_token).balanceOf(address(this)));
91    }
92    // reset pending request
93    delete rescueRequests[_token];
94    // emit Rescue(_token, req.receiver, block.timestamp);
95}

Recommendation:

As the rescueRequests entry is not utilized beyond the AsRescuable::_isRescueUnlocked validation, we advise the entry to be deleted immediately after validation, optimizing the code's gas cost.

Alleviation (cf5194da53ebf026da6c8efa74daada96719cc71):

The inefficiency has been addressed by issuing the delete operation in place of the timestamp erasure statement, optimizing the code's gas cost.

ARE-03C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		AsRescuable.sol:L79, L84

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
79RescueRequest storage req = rescueRequests[_token];
80// check if rescue is pending
81require(_isRescueUnlocked(req));
82
83// reset timestamp to prevent reentrancy
84rescueRequests[_token].timestamp = 0;

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

The second highlighted instance properly utilizes the existing req storage pointer, optimizing the code as advised.

ARE-04C: Inexistent Error Messages

Type	Severity	Location
Code Style		AsRescuable.sol:L57, L81

Description:

The linked require checks have no error messages explicitly defined.

Example:

Copy
57require(!_isRescueUnlocked(req));

Recommendation:

We advise each to be set so to increase the legibility of the codebase and aid in validating the require checks' conditions.

Alleviation (59b75fbee1d8f3dee807c928f18be41c58b904e1):

While the latter of the two require checks is accompanied by descriptive in-line documentation, the former is not thus rendering this exhibit acknowledged.