Liquidity Management

Details:
In the _handleReturn function of the PerpetualVault contract, the Burned event is emitted before the _burn function is called to delete the deposit information. This ordering means that the event log shows deposit data (such as the number of shares and recipient) that is still present in storage at the time of emission. Off-chain systems or indexers relying on these events to reconstruct the vault’s state may see inconsistent or outdated information.

Root Cause:
The function emits the Burned event prior to calling _burn(depositId), which updates the internal state by removing the deposit record. This order of operations does not follow the best practice of logging state changes after the state has been updated.

Impact:

• Off-Chain Inconsistency: External systems and monitoring tools that depend on event logs for state synchronization could misinterpret the vault’s state, leading to inaccurate analytics or reporting.

• Developer Confusion: Future developers or auditors reviewing the logs might be misled about the true state of deposits, which can complicate troubleshooting and system maintenance.

Recommendation:
Reorder the operations in _handleReturn so that the _burn(depositId) call is executed before emitting the Burned event. This will ensure that the event log accurately reflects the state change (i.e., the deposit record has been removed) at the time the event is emitted. For example:

Alternatively, if the event requires data from depositInfo before deletion, cache the necessary values in local variables and then emit the event after burning the deposit.

Proof of Concept:

1. A deposit exists with a nonzero number of shares stored in depositInfo.

2. The _handleReturn function is triggered for a withdrawal, and the contract calculates the amount to be returned.

3. The _transferToken function is called, and then the Burned event is emitted using values directly from depositInfo.

4. After the event emission, the _burn(depositId) call deletes the deposit data.

5. Off-chain indexers capturing the Burned event will see the deposit information as if it were still valid, despite the state having changed immediately afterward, leading to a temporary inconsistency in the off-chain representation of the vault’s state.

By adjusting the event emission order, the event will accurately reflect that the deposit has been removed, ensuring consistency between on-chain state and off-chain logs.