finalize request safeguards

src/WithdrawRequestNFT.sol

@@ -11,8 +11,6 @@ import "./interfaces/IWithdrawRequestNFT.sol";
 import "./interfaces/IMembershipManager.sol";
 import "./RoleRegistry.sol";
 
-import "forge-std/console.sol";
-
 contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgradeable, IWithdrawRequestNFT {
     //--------------------------------------------------------------------------------------
     //---------------------------------  STATE-VARIABLES  ----------------------------------
@@ -153,6 +151,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad
 
     /// @notice finalizes a batch of requests and locks the corresponding ETH to be withdrawn
     /// @dev called by the `EtherFiAdmin` contract to finalize a batch of requests based on the last oracle report 
+    /// @param lastRequestId the id of the last request to finalize in this batch, will update `lastFinalizedRequestId` value
     function finalizeRequests(uint32 lastRequestId) external {
         if (!roleRegistry.hasRole(WITHDRAW_NFT_ADMIN_ROLE, msg.sender)) revert IncorrectRole();
 
@@ -162,8 +161,14 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad
 
     /// @notice `finalizeRequests` with the ability to specify the total amount of ETH to be locked
     /// @dev The oracle calculates the amount of ETH that is needed to fulfill the pending withdrawal off-chain
+    /// @param lastRequestId the id of the last request to finalize in this batch, will update `lastFinalizedRequestId` value
+    /// @param totalAmount the total amount of ETH to be locked for the requests in this batch
     function finalizeRequests(uint256 lastRequestId, uint256 totalAmount) external {
         if (!roleRegistry.hasRole(WITHDRAW_NFT_ADMIN_ROLE, msg.sender)) revert IncorrectRole();
+        require(lastRequestId >= lastFinalizedRequestId, "Invalid lastRequestId submitted");
+
+        // No new requests have been finalized since the last oracle report
+        if (lastRequestId == lastFinalizedRequestId) { return; }
 
         _finalizeRequests(lastRequestId, totalAmount);
     }
@@ -199,7 +204,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad
     /// `checkpointIndex` can be found using `findCheckpointIndex()` function
     /// @return uint256 the amount of ETH that can be claimed by the owner of the NFT
     function getClaimableAmount(uint32 requestId, uint32 checkpointIndex) public view returns (uint256) {
-        require(isFinalized(tokenId), "Request is not finalized");
+        require(isFinalized(requestId), "Request is not finalized");
         require(requestId < nextRequestId, "Request does not exist");
         require(ownerOf(requestId) != address(0), "Already claimed");
 
@@ -286,10 +291,14 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad
     /// @notice The amount of eETH that needed to fulfill the pending withdrawal requests up to and including `lastRequestId`
     function calculateTotalPendingAmount(uint32 lastRequestId) public view returns (uint256) {
         uint256 totalAmount = 0;
+        uint256 preciseSharePrice = liquidityPool.amountForShare(E27_PRECISION_BASE);
+
         for (uint32 i = lastFinalizedRequestId + 1; i <= lastRequestId; i++) {
 
             IWithdrawRequestNFT.WithdrawRequest memory request = _requests[i];
-            uint256 amountForShares = liquidityPool.amountForShare(request.shareOfEEth);
+
+            // Use the precise share price calculation to maintain conistency with how amount is calculated in `getClaimableAmount`
+            uint256 amountForShares = request.shareOfEEth * preciseSharePrice / E27_PRECISION_BASE;
             uint256 amount = _min(request.amountOfEEth, amountForShares);
 
             totalAmount += amount;
@@ -350,9 +359,7 @@ contract WithdrawRequestNFT is ERC721Upgradeable, UUPSUpgradeable, OwnableUpgrad
 
         lastFinalizedRequestId = uint32(lastRequestId);
 
-        if (totalAmount > 0) {
-            liquidityPool.withdraw(address(this), totalAmount);
-        }
+        liquidityPool.withdraw(address(this), totalAmount);
 
         emit UpdateFinalizedRequestId(uint32(lastRequestId), totalAmount);
     }