ERC721PortalFacet Manual Review Findings

ERC-01M: Potential Slippage Argument

Description:

The burnERC721 function may result in a discrepant fee being charged to the user as the fee can change between a user's transaction submission and execution.

Example:

Copy
72function burnERC721(
73    uint256 _targetChain,
74    address _wrappedToken,
75    uint256 _tokenId,
76    bytes memory _receiver
77) public override whenNotPaused {
78    address payment = LibERC721.erc721Payment(_wrappedToken);
79    require(
80        LibPayment.containsPaymentToken(payment),
81        "ERC721PortalFacet: payment token not supported"
82    );
83    uint256 fee = LibERC721.erc721Fee(_wrappedToken);
84
85    IERC20(payment).safeTransferFrom(msg.sender, address(this), fee);
86    LibFeeCalculator.accrueFee(payment, fee);
87
88    WrappedERC721(_wrappedToken).burn(_tokenId);
89    emit BurnERC721(_targetChain, _wrappedToken, _tokenId, _receiver, fee);
90}

Recommendation:

We advise a form of slippage argument to be introduced whereby the maximum fee the user is willing to pay is enforced via a require check.

Alleviation:

The issue was remediated by introducing two additional arguments to the burnERC721 function that allow the user to specify the _fee they are willing to pay as well as the _paymentToken they want to pay in both of which are mandated to match exactly within the function.