refactor: addressing internal audit - #216
Conversation
kupermind
commented
Jun 23, 2025
kupermind
commented
- Addressing internal audit.
refactor: account for withheld amount update flag
| /// @dev Processes the data received from L1. | ||
| /// @param data Bytes message data sent from L1. | ||
| function _processData(bytes memory data) internal { | ||
| function _processData(bytes memory data) internal returns (uint256 totalAmount) { |
There was a problem hiding this comment.
Since proposed deposited amounts could be further cut off by the StakingFactory verification and result in more withheldAmount-s, let's compute the total amount that is deposited.
| // Update total to-be-deposited amount | ||
| totalAmount += amount; |
There was a problem hiding this comment.
Add up to the total amount to-be-deposited
| // Process the data and calculate deposited amounts | ||
| uint256 totalAmount = _processData(data); | ||
|
|
||
| // Update withheld amount | ||
| if (updateWithheldAmount) { | ||
| uint256 localWithheldAmount = withheldAmount; | ||
|
|
||
| // Check for overflow | ||
| if (totalAmount > localWithheldAmount) { | ||
| revert Overflow(totalAmount, localWithheldAmount); | ||
| } | ||
|
|
||
| // Update withheld amount | ||
| localWithheldAmount -= totalAmount; | ||
| withheldAmount = localWithheldAmount; | ||
|
|
||
| emit WithheldAmountUpdated(localWithheldAmount); |
There was a problem hiding this comment.
Calculate total amount being deposited from the balance of this contract. If the balance is used that is recorded in withheldAmount value, then it's the exact amount we need to subtract from withheldAmount.
There was a problem hiding this comment.
I am not sure why there should be an explicit calculation here, we had this implemented in such a way the DAO needs to take care to add correct numbers. Let's sync tomorrow on this
There was a problem hiding this comment.
I approve this, since we discussed and converged on this approach
| // Check for the contract ownership | ||
| if (msg.sender != owner) { | ||
| revert OwnerOnly(msg.sender, owner); | ||
| } |
There was a problem hiding this comment.
Adding owner check as suggested by the audit.
doc: re-audit 1.3.3
chore: Addressing internal audit 2
2 participants
