DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

[M-3] `ExecutionFeeRefundFailed` event is never emitted in `PerpetualVault.sol`

Description

The ExecutionFeeRefundFailed event is defined but is never emitted in the catch blocks of the refund fee as the Protocol itself depends upon Many Offchain functionalities the Event Logs play a major part in this case and not emitting the events will make it hard to trace the failed refund transactions

Impact

Usually this would be a low issue but as the protocol is heavily dependent upon off-chain functionalities not having proper event logs will cause discrepancies i.e In case of multiple Execution Fee Refund failures it would be hard to trace the failed transactions hence this will would be a medium in this case

Proof of Concept

In the PerpetualVault::_mint function , the event is not emitted

if (refundFee) {
console.log("hits this");
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[counter].executionFee > usedFee) {
console.log("Refund 1 Owner: ", depositInfo[counter].owner);
console.log("Refund 1 : ", depositInfo[counter].executionFee - usedFee);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
) {} catch {
@>
}
}
}
emit Minted(depositId, depositInfo[depositId].owner, _shares, amount);

In the PerpetualVault::_handleReturn function , the event is not emitted

emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
@> ) {} catch {
}
}
}
// update global state
delete swapProgressData;
delete flowData;
delete flow;
}

In the PerpetualVault::_cancelFlow function the event is not emitted

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
@> ) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
@> ) {} catch {}
}

Tools Used

Manual Review

Recommendations

Emit the ExecutionFeeRefundFailed event in the refund fee catch blocks of PerpetualVault::_mint, PerpetualVault::_cancelFlow and PerpetualVault::_handleReturn functions

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!