re-al-Foundation / rwa-contracts

0 stars 0 forks source link

[DFY-02C] Inefficient EIP-721 Transfer Flow #79

Closed chasebrownn closed 5 months ago

chasebrownn commented 5 months ago

DFY-02C: Inefficient EIP-721 Transfer Flow

Type Severity Location
Gas Optimization DelegateFactory.sol:L106, L125-L126

Description:

The DelegateFactory::deployDelegator will transfer the _tokenId it is meant to deposit to the delegator inward from the msg.sender, will approve the Delegator deployed, and the Delegator::depositDelegatorToken function will transfer the token from the DelegateFactory to the Delegator.

Example:

/**
 * @notice This method allows a permissioned address to create a delegator contract.
 * @param _tokenId Token identifier for NFT being delegated.
 * @param _delegatee Address to which the voting power of `_tokenId` is delegated.
 * @param _duration Amount of time to delegate voting power. Will be the expiration date for the new Delegator.
 * @return newDelegator -> Contract address of new Delegator beacon proxy.
 */
function deployDelegator(uint256 _tokenId, address _delegatee, uint256 _duration) external returns (address newDelegator) {
    require(canDelegate[msg.sender] || msg.sender == owner(), "DelegateFactory: Not authorized");

    // take token
    veRWA.safeTransferFrom(msg.sender, address(this), _tokenId);

    // create delegator
    FetchableBeaconProxy newDelegatorBeacon = new FetchableBeaconProxy(
        address(beacon),
        abi.encodeWithSelector(Delegator.initialize.selector,
            address(veRWA),
            msg.sender,
            _delegatee
        )
    );

    newDelegator = address(newDelegatorBeacon);

    delegators.push(newDelegator);
    isDelegator[newDelegator] = true;
    delegatorExpiration[newDelegator] = block.timestamp + _duration;

    // deposit token into delegator
    veRWA.approve(newDelegator, _tokenId);
    (bool success,) = newDelegator.call(abi.encodeWithSignature("depositDelegatorToken(uint256)", _tokenId));
    require(success, "deposit unsuccessful");

    emit DelegatorCreated(newDelegator);
}

Recommendation:

We advise the EIP-721 asset to be transferred to the Delegator directly, either by the Delegator::depositDelegatorToken function or the DelegateFactory::deployDelegator function.

chasebrownn commented 5 months ago

Acknowledged