From ff4a9e1dd259e7253790691a98a05da806e1d0fd Mon Sep 17 00:00:00 2001 From: Xi Lin Date: Thu, 20 Jul 2023 16:10:06 +0800 Subject: [PATCH] fix(contracts): OZ-L1-L07 Lack of Logs on Sensitive Actions (#623) Co-authored-by: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com> --- contracts/src/libraries/FeeVault.sol | 65 ++++++++++++++++--- .../token/IScrollStandardERC20Factory.sol | 3 + .../token/ScrollStandardERC20Factory.sol | 6 +- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/contracts/src/libraries/FeeVault.sol b/contracts/src/libraries/FeeVault.sol index 5bf0805f4..853c5092b 100644 --- a/contracts/src/libraries/FeeVault.sol +++ b/contracts/src/libraries/FeeVault.sol @@ -28,10 +28,17 @@ pragma solidity ^0.8.0; import {IL2ScrollMessenger} from "../L2/IL2ScrollMessenger.sol"; import {OwnableBase} from "./common/OwnableBase.sol"; +// solhint-disable no-empty-blocks +// solhint-disable reason-string + /// @title FeeVault /// @notice The FeeVault contract contains the basic logic for the various different vault contracts /// used to hold fee revenue generated by the L2 system. abstract contract FeeVault is OwnableBase { + /********** + * Events * + **********/ + /// @notice Emits each time that a withdrawal occurs. /// /// @param value Amount that was withdrawn (in wei). @@ -39,6 +46,25 @@ abstract contract FeeVault is OwnableBase { /// @param from Address that triggered the withdrawal. event Withdrawal(uint256 value, address to, address from); + /// @notice Emits each time the owner updates the address of `messenger`. + /// @param oldMessenger The address of old messenger. + /// @param newMessenger The address of new messenger. + event UpdateMessenger(address indexed oldMessenger, address indexed newMessenger); + + /// @notice Emits each time the owner updates the address of `recipient`. + /// @param oldRecipient The address of old recipient. + /// @param newRecipient The address of new recipient. + event UpdateRecipient(address indexed oldRecipient, address indexed newRecipient); + + /// @notice Emits each time the owner updates the value of `minWithdrawAmount`. + /// @param oldMinWithdrawAmount The value of old `minWithdrawAmount`. + /// @param newMinWithdrawAmount The value of new `minWithdrawAmount`. + event UpdateMinWithdrawAmount(uint256 oldMinWithdrawAmount, uint256 newMinWithdrawAmount); + + /************* + * Variables * + *************/ + /// @notice Minimum balance before a withdrawal can be triggered. uint256 public minWithdrawAmount; @@ -51,6 +77,10 @@ abstract contract FeeVault is OwnableBase { /// @notice Total amount of wei processed by the contract. uint256 public totalProcessed; + /*************** + * Constructor * + ***************/ + /// @param _owner The owner of the contract. /// @param _recipient Wallet that will receive the fees on L1. /// @param _minWithdrawalAmount Minimum balance before a withdrawal can be triggered. @@ -65,6 +95,10 @@ abstract contract FeeVault is OwnableBase { recipient = _recipient; } + /***************************** + * Public Mutating Functions * + *****************************/ + /// @notice Allow the contract to receive ETH. receive() external payable {} @@ -92,21 +126,34 @@ abstract contract FeeVault is OwnableBase { ); } + /************************ + * Restricted Functions * + ************************/ + /// @notice Update the address of messenger. - /// @param _messenger The address of messenger to update. - function updateMessenger(address _messenger) external onlyOwner { - messenger = _messenger; + /// @param _newMessenger The address of messenger to update. + function updateMessenger(address _newMessenger) external onlyOwner { + address _oldMessenger = messenger; + messenger = _newMessenger; + + emit UpdateMessenger(_oldMessenger, _newMessenger); } /// @notice Update the address of recipient. - /// @param _recipient The address of recipient to update. - function updateRecipient(address _recipient) external onlyOwner { - recipient = _recipient; + /// @param _newRecipient The address of recipient to update. + function updateRecipient(address _newRecipient) external onlyOwner { + address _oldRecipient = recipient; + recipient = _newRecipient; + + emit UpdateRecipient(_oldRecipient, _newRecipient); } /// @notice Update the minimum withdraw amount. - /// @param _minWithdrawAmount The minimum withdraw amount to update. - function updateMinWithdrawAmount(uint256 _minWithdrawAmount) external onlyOwner { - minWithdrawAmount = _minWithdrawAmount; + /// @param _newMinWithdrawAmount The minimum withdraw amount to update. + function updateMinWithdrawAmount(uint256 _newMinWithdrawAmount) external onlyOwner { + uint256 _oldMinWithdrawAmount = minWithdrawAmount; + minWithdrawAmount = _newMinWithdrawAmount; + + emit UpdateMinWithdrawAmount(_oldMinWithdrawAmount, _newMinWithdrawAmount); } } diff --git a/contracts/src/libraries/token/IScrollStandardERC20Factory.sol b/contracts/src/libraries/token/IScrollStandardERC20Factory.sol index 605862c8f..9bd34bc1d 100644 --- a/contracts/src/libraries/token/IScrollStandardERC20Factory.sol +++ b/contracts/src/libraries/token/IScrollStandardERC20Factory.sol @@ -3,6 +3,9 @@ pragma solidity ^0.8.0; interface IScrollStandardERC20Factory { + /// @notice Emitted when a l2 token is deployed. + /// @param _l1Token The address of the l1 token. + /// @param _l2Token The address of the l2 token. event DeployToken(address indexed _l1Token, address indexed _l2Token); /// @notice Compute the corresponding l2 token address given l1 token address. diff --git a/contracts/src/libraries/token/ScrollStandardERC20Factory.sol b/contracts/src/libraries/token/ScrollStandardERC20Factory.sol index 5c10c636d..48d2912f6 100644 --- a/contracts/src/libraries/token/ScrollStandardERC20Factory.sol +++ b/contracts/src/libraries/token/ScrollStandardERC20Factory.sol @@ -35,7 +35,11 @@ contract ScrollStandardERC20Factory is Ownable, IScrollStandardERC20Factory { function deployL2Token(address _gateway, address _l1Token) external onlyOwner returns (address) { bytes32 _salt = _getSalt(_gateway, _l1Token); - return Clones.cloneDeterministic(implementation, _salt); + address _l2Token = Clones.cloneDeterministic(implementation, _salt); + + emit DeployToken(_l1Token, _l2Token); + + return _l2Token; } function _getSalt(address _gateway, address _l1Token) internal pure returns (bytes32) {