Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create SecurityMonitorFacet and add associated mock and tests #954

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"url": "https://github.com/ubiquity/ubiquity-dollar.git"
},
"dependencies": {
"@chainlink/contracts": "^1.2.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this package used anywhere? If not then it should be removed, chainlink contracts are already added as a github submodule here.

"@types/command-line-args": "5.2.0",
"command-line-args": "5.2.1",
"dotenv": "^16.0.3",
Expand Down
163 changes: 163 additions & 0 deletions packages/contracts/src/dollar/facets/SecurityMonitorFacet.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.19;

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/utils/math/SafeMath.sol";
import "../../../src/dollar/libraries/LibDiamond.sol";
import "../../../src/dollar/libraries/LibAccessControl.sol";
import "../../../lib/chainlink-brownie-contracts/contracts/src/v0.8/AutomationCompatible.sol";
import "../../../src/dollar/interfaces/IUbiquityPool.sol";
import "../../../src/dollar/interfaces/IUbiquityDollarToken.sol";
import "../../../src/dollar/interfaces/ITelegramNotifier.sol";

contract SecurityMonitorFacet is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security monitor contract should not be a part of the diamond (i.e. a separate facet). We'd keep things simple and have a separate SecurityMonitor contract which we'd deploy manually.

ReentrancyGuard,
AutomationCompatibleInterface
{
using SafeMath for uint256;

bytes32 public constant SECURITY_MONITOR_ROLE =
keccak256("SECURITY_MONITOR_ROLE");
bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

uint256 public constant LIQUIDITY_THRESHOLD = 70; // 70% of initial liquidity
uint256 public constant COLLATERAL_RATIO_THRESHOLD = 900000; // 90% (using 1e6 precision)
uint256 public constant LARGE_TRANSFER_THRESHOLD = 1000000e18; // 1 million dollars

struct SecurityMonitorStorage {
uint256 lastCheckTimestamp;
uint256 checkInterval;
}

bytes32 constant SECURITY_MONITOR_STORAGE_POSITION =
keccak256("security.monitor.storage");

function securityMonitorStorage()
internal
pure
returns (SecurityMonitorStorage storage s)
{
bytes32 position = SECURITY_MONITOR_STORAGE_POSITION;
assembly {
s.slot := position
}
}

event SecurityIncident(string message);

function initialize(uint256 _checkInterval) external {
LibDiamond.enforceIsContractOwner();
SecurityMonitorStorage storage s = securityMonitorStorage();
s.checkInterval = _checkInterval;
s.lastCheckTimestamp = block.timestamp;
}

function checkUpkeep(
bytes calldata /* checkData */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to use log based upkeeps? Then we can catch transactions that look malicious right when they occur instead of relying on a perfect timing.

)
external
view
override
returns (bool upkeepNeeded, bytes memory performData)
{
SecurityMonitorStorage storage s = securityMonitorStorage();

upkeepNeeded =
(block.timestamp - s.lastCheckTimestamp) >= s.checkInterval;

performData = ""; // Return an empty bytes array
}

function performUpkeep(bytes calldata /* performData */) external override {
SecurityMonitorStorage storage s = securityMonitorStorage();

if ((block.timestamp - s.lastCheckTimestamp) >= s.checkInterval) {
s.lastCheckTimestamp = block.timestamp;

checkSecurityConditions();
}
}

function checkSecurityConditions() internal {
if (checkLiquidity()) {
handleSecurityIncident("Liquidity below threshold");
}

if (checkCollateralRatio()) {
handleSecurityIncident("Collateral ratio below threshold");
}

// Add more security checks as needed
}

function handleSecurityIncident(
string memory message
) internal nonReentrant {
require(
LibAccessControl.hasRole(SECURITY_MONITOR_ROLE, address(this)),
"SecurityMonitor: not authorized"
);

// Pause the UbiquityDollarToken and UbiquityPoolFacet

if (LibAccessControl.hasRole(PAUSER_ROLE, address(this))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check in unnecessary since we'll grant the PAUSER role to the security contract.

// Instead of directly calling pause(), we'll use a more generic approach

// that should work regardless of how pausing is implemented in UbiquityDollarToken

(bool success, ) = address(LibDiamond.contractOwner()).call(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try that it works as expected? Contract owner is an EOA so I don't fully understand how we can call the pauseDollarToken() method on it.

abi.encodeWithSignature("pauseDollarToken()")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no such method pauseDollarToken() in the UbiquityDollarToken contract.

);

require(success, "Failed to pause UbiquityDollarToken");

IUbiquityPool(LibDiamond.contractOwner()).toggleMintRedeemBorrow(
0,
0
); // Pause minting

IUbiquityPool(LibDiamond.contractOwner()).toggleMintRedeemBorrow(
0,
1
); // Pause redeeming

emit SecurityIncident(message);

ITelegramNotifier(LibDiamond.contractOwner()).notify(message);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this line. I actually thought that services like https://defender.openzeppelin.com/ has a feature of sending telegram messages that can be set up in a few clicks. Why do we need the ITelegramNotifier interface and how is it supposed to work?

} else {
emit SecurityIncident(
"Failed to pause: SecurityMonitor lacks PAUSER_ROLE"
);

ITelegramNotifier(LibDiamond.contractOwner()).notify(
"Failed to pause: SecurityMonitor lacks PAUSER_ROLE"
);
}
}

function checkLiquidity() internal view returns (bool) {
uint256 poolLiquidity = IUbiquityPool(LibDiamond.contractOwner())
.collateralUsdBalance();

uint256 thresholdLiquidity = poolLiquidity.mul(LIQUIDITY_THRESHOLD).div(
100
);

return poolLiquidity < thresholdLiquidity;
}

function checkCollateralRatio() internal view returns (bool) {
uint256 currentRatio = IUbiquityPool(LibDiamond.contractOwner())
.collateralRatio();

return currentRatio < COLLATERAL_RATIO_THRESHOLD;
}

function setCheckInterval(uint256 _newInterval) external {
LibDiamond.enforceIsContractOwner();

SecurityMonitorStorage storage s = securityMonitorStorage();

s.checkInterval = _newInterval;
}
}
15 changes: 15 additions & 0 deletions packages/contracts/src/dollar/interfaces/ITelegramNotifier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

interface ITelegramNotifier {
/**

* @notice Sends a notification message via Telegram

* @param message The message to be sent

*/

function notify(string memory message) external;
}
27 changes: 27 additions & 0 deletions packages/contracts/src/dollar/mocks/MockTelegramNotifier.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "../interfaces/ITelegramNotifier.sol";

contract MockTelegramNotifier is ITelegramNotifier {
event NotificationSent(string message);

bool public lastNotificationSent;

string public lastNotificationMessage;

function notify(string memory message) external override {
lastNotificationSent = true;

lastNotificationMessage = message;

emit NotificationSent(message);
}

function resetNotification() external {
lastNotificationSent = false;

lastNotificationMessage = "";
}
}
Loading