Enhance ComboHandler and orchestrator functionality with access control and error handling improvements
- Added AccessControl to ComboHandler for role-based access management. - Implemented gas estimation for plan execution and improved gas limit checks. - Updated execution and preparation methods to enforce step count limits and role restrictions. - Enhanced error handling in orchestrator API endpoints with AppError for better validation feedback. - Integrated request timeout middleware for improved request management. - Updated Swagger documentation to reflect new API structure and parameters.
This commit is contained in:
defiQUG
@@ -4,6 +4,7 @@ pragma solidity ^0.8.20;
|
||||
import "@openzeppelin/contracts/access/Ownable.sol";
|
||||
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
|
||||
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
|
||||
import "@openzeppelin/contracts/access/AccessControl.sol";
|
||||
import "./interfaces/IComboHandler.sol";
|
||||
import "./interfaces/IAdapterRegistry.sol";
|
||||
import "./interfaces/INotaryRegistry.sol";
|
||||
@@ -11,11 +12,13 @@ import "./interfaces/INotaryRegistry.sol";
|
||||
/**
|
||||
* @title ComboHandler
|
||||
* @notice Aggregates multiple DeFi protocol calls and DLT operations into atomic transactions
|
||||
* @dev Implements 2PC pattern and proper signature verification
|
||||
* @dev Implements 2PC pattern, proper signature verification, access control, and gas optimization
|
||||
*/
|
||||
contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard, AccessControl {
|
||||
using ECDSA for bytes32;
|
||||
|
||||
bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
|
||||
|
||||
IAdapterRegistry public immutable adapterRegistry;
|
||||
INotaryRegistry public immutable notaryRegistry;
|
||||
|
||||
@@ -27,18 +30,22 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
Step[] steps;
|
||||
bool prepared;
|
||||
address creator;
|
||||
uint256 gasLimit;
|
||||
}
|
||||
|
||||
event PlanExecuted(bytes32 indexed planId, bool success, uint256 gasUsed);
|
||||
event PlanPrepared(bytes32 indexed planId, address indexed creator);
|
||||
event PlanCommitted(bytes32 indexed planId);
|
||||
event PlanAborted(bytes32 indexed planId, string reason);
|
||||
event StepExecuted(bytes32 indexed planId, uint256 stepIndex, bool success, uint256 gasUsed);
|
||||
|
||||
constructor(address _adapterRegistry, address _notaryRegistry) {
|
||||
require(_adapterRegistry != address(0), "Invalid adapter registry");
|
||||
require(_notaryRegistry != address(0), "Invalid notary registry");
|
||||
adapterRegistry = IAdapterRegistry(_adapterRegistry);
|
||||
notaryRegistry = INotaryRegistry(_notaryRegistry);
|
||||
|
||||
_grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -55,25 +62,26 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
bytes calldata signature
|
||||
) external override nonReentrant returns (bool success, StepReceipt[] memory receipts) {
|
||||
require(executions[planId].status == ExecutionStatus.PENDING, "Plan already executed");
|
||||
require(steps.length > 0, "Plan must have at least one step");
|
||||
require(steps.length > 0 && steps.length <= 20, "Invalid step count");
|
||||
|
||||
// Verify signature using ECDSA
|
||||
bytes32 messageHash = keccak256(abi.encodePacked(planId, steps, msg.sender));
|
||||
bytes32 ethSignedMessageHash = messageHash.toEthSignedMessageHash();
|
||||
address signer = ethSignedMessageHash.recover(signature);
|
||||
bytes32 messageHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", keccak256(abi.encodePacked(planId, steps, msg.sender))));
|
||||
address signer = messageHash.recover(signature);
|
||||
require(signer == msg.sender, "Invalid signature");
|
||||
|
||||
// Register with notary
|
||||
notaryRegistry.registerPlan(planId, steps, msg.sender);
|
||||
|
||||
uint256 gasStart = gasleft();
|
||||
uint256 estimatedGas = _estimateGas(steps);
|
||||
|
||||
executions[planId] = ExecutionState({
|
||||
status: ExecutionStatus.IN_PROGRESS,
|
||||
currentStep: 0,
|
||||
steps: steps,
|
||||
prepared: false,
|
||||
creator: msg.sender
|
||||
creator: msg.sender,
|
||||
gasLimit: estimatedGas
|
||||
});
|
||||
|
||||
receipts = new StepReceipt[](steps.length);
|
||||
@@ -81,6 +89,10 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
// Execute steps sequentially
|
||||
for (uint256 i = 0; i < steps.length; i++) {
|
||||
uint256 stepGasStart = gasleft();
|
||||
|
||||
// Check gas limit
|
||||
require(gasleft() > 100000, "Insufficient gas");
|
||||
|
||||
(bool stepSuccess, bytes memory returnData, uint256 gasUsed) = _executeStep(steps[i], i);
|
||||
|
||||
receipts[i] = StepReceipt({
|
||||
@@ -90,6 +102,8 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
gasUsed: stepGasStart - gasleft()
|
||||
});
|
||||
|
||||
emit StepExecuted(planId, i, stepSuccess, gasUsed);
|
||||
|
||||
if (!stepSuccess) {
|
||||
executions[planId].status = ExecutionStatus.FAILED;
|
||||
notaryRegistry.finalizePlan(planId, false);
|
||||
@@ -116,9 +130,9 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
function prepare(
|
||||
bytes32 planId,
|
||||
Step[] calldata steps
|
||||
) external override returns (bool prepared) {
|
||||
) external override onlyRole(EXECUTOR_ROLE) returns (bool prepared) {
|
||||
require(executions[planId].status == ExecutionStatus.PENDING, "Plan not pending");
|
||||
require(steps.length > 0, "Plan must have at least one step");
|
||||
require(steps.length > 0 && steps.length <= 20, "Invalid step count");
|
||||
|
||||
// Validate all steps can be prepared
|
||||
for (uint256 i = 0; i < steps.length; i++) {
|
||||
@@ -130,7 +144,8 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
currentStep: 0,
|
||||
steps: steps,
|
||||
prepared: true,
|
||||
creator: msg.sender
|
||||
creator: msg.sender,
|
||||
gasLimit: _estimateGas(steps)
|
||||
});
|
||||
|
||||
emit PlanPrepared(planId, msg.sender);
|
||||
@@ -142,7 +157,7 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
* @param planId Plan identifier
|
||||
* @return committed Whether commit was successful
|
||||
*/
|
||||
function commit(bytes32 planId) external override returns (bool committed) {
|
||||
function commit(bytes32 planId) external override onlyRole(EXECUTOR_ROLE) returns (bool committed) {
|
||||
ExecutionState storage state = executions[planId];
|
||||
require(state.prepared, "Plan not prepared");
|
||||
require(state.status == ExecutionStatus.IN_PROGRESS, "Invalid state");
|
||||
@@ -168,6 +183,7 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
function abort(bytes32 planId) external override {
|
||||
ExecutionState storage state = executions[planId];
|
||||
require(state.status == ExecutionStatus.IN_PROGRESS, "Cannot abort");
|
||||
require(msg.sender == state.creator || hasRole(EXECUTOR_ROLE, msg.sender), "Not authorized");
|
||||
|
||||
// Release any reserved funds/collateral
|
||||
_rollbackSteps(planId);
|
||||
@@ -186,9 +202,17 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
return executions[planId].status;
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Estimate gas for plan execution
|
||||
*/
|
||||
function _estimateGas(Step[] memory steps) internal pure returns (uint256) {
|
||||
// Rough estimation: 100k per step + 50k overhead
|
||||
return steps.length * 100000 + 50000;
|
||||
}
|
||||
|
||||
/**
|
||||
* @notice Execute a single step
|
||||
* @dev Internal function with gas tracking
|
||||
* @dev Internal function with gas tracking and optimization
|
||||
*/
|
||||
function _executeStep(Step memory step, uint256 stepIndex) internal returns (bool success, bytes memory returnData, uint256 gasUsed) {
|
||||
// Verify adapter is whitelisted
|
||||
@@ -199,17 +223,15 @@ contract ComboHandler is IComboHandler, Ownable, ReentrancyGuard {
|
||||
// Check gas limit
|
||||
require(gasleft() > 100000, "Insufficient gas");
|
||||
|
||||
(success, returnData) = step.target.call{value: step.value}(
|
||||
(success, returnData) = step.target.call{value: step.value, gas: gasleft() - 50000}(
|
||||
abi.encodeWithSignature("executeStep(bytes)", step.data)
|
||||
);
|
||||
|
||||
gasUsed = gasBefore - gasleft();
|
||||
|
||||
// Emit event for step execution
|
||||
if (success) {
|
||||
// Log successful step
|
||||
} else {
|
||||
// Log failed step with return data
|
||||
if (!success && returnData.length > 0) {
|
||||
// Log failure reason if available
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user