Merge pull request #142 from PracticalParticle/dev

Dev

Author: JaCoderX

abi/GuardControllerDefinitions.abi.json

@@ -51,6 +51,19 @@
     "stateMutability": "view",
     "type": "function"
   },
+  {
+    "inputs": [],
+    "name": "CONTROLLER_CONFIG_OPERATION",
+    "outputs": [
+      {
+        "internalType": "bytes32",
+        "name": "",
+        "type": "bytes32"
+      }
+    ],
+    "stateMutability": "view",
+    "type": "function"
+  },
   {
     "inputs": [],
     "name": "CONTROLLER_OPERATION",

contracts/core/access/RuntimeRBAC.sol

@@ -134,7 +134,7 @@ abstract contract RuntimeRBAC is BaseStateMachine, IRuntimeRBAC {
     function _executeRoleConfigBatch(IRuntimeRBAC.RoleConfigAction[] calldata actions) internal {
         _validateBatchSize(actions.length);
 
-        for (uint256 i = 0; i < actions.length; i++) {
+        for (uint256 i = 0; i < actions.length; ++i) {
             IRuntimeRBAC.RoleConfigAction calldata action = actions[i];
 
             if (action.actionType == IRuntimeRBAC.RoleConfigActionType.CREATE_ROLE) {

contracts/core/execution/lib/definitions/GuardControllerDefinitions.sol

@@ -16,7 +16,7 @@ import "../../interface/IGuardController.sol";
  * and role permissions for GuardController's public execution functions.
  * 
  * Key Features:
- * - Registers all 6 GuardController public execution functions
+ * - Registers all 9 GuardController public execution functions
  * - Defines role permissions for OWNER_ROLE and BROADCASTER_ROLE
  * - Supports time-delay and meta-transaction workflows
  * - Matches EngineBloxDefinitions pattern for consistency
@@ -33,6 +33,8 @@ library GuardControllerDefinitions {
 
     // Operation Type Constants
     bytes32 public constant CONTROLLER_OPERATION = keccak256("CONTROLLER_OPERATION");
+    // Guard config batch only (whitelist / register-unregister function); distinct execution operation type bitmap.
+    bytes32 public constant CONTROLLER_CONFIG_OPERATION = keccak256("CONTROLLER_CONFIG_OPERATION");
 
     // Function Selector Constants
     // GuardController: executeWithTimeLock(address,uint256,bytes4,bytes,uint256,bytes32)
@@ -86,7 +88,7 @@ library GuardControllerDefinitions {
      * 
      * Function schemas define:
      * - GuardController public execution functions
-     * - What operation types they belong to (CONTROLLER_OPERATION)
+     * - What operation types they belong to (CONTROLLER_OPERATION vs CONTROLLER_CONFIG_OPERATION)
      * - What actions are supported (time-delay request/approve/cancel, meta-tx approve/cancel/request-and-approve)
      * - Whether they are protected
      * 
@@ -96,7 +98,7 @@ library GuardControllerDefinitions {
      * - Role permissions are defined in getRolePermissions() matching EngineBloxDefinitions pattern
      */
     function getFunctionSchemas() public pure returns (EngineBlox.FunctionSchema[] memory) {
-        EngineBlox.FunctionSchema[] memory schemas = new EngineBlox.FunctionSchema[](8);
+        EngineBlox.FunctionSchema[] memory schemas = new EngineBlox.FunctionSchema[](9);
 
         // ============ TIME-DELAY WORKFLOW ACTIONS ============
         // Request action for executeWithTimeLock
@@ -144,6 +146,9 @@ library GuardControllerDefinitions {
         requestAndApproveExecutionHandlerForSelectors[0] = REQUEST_AND_APPROVE_EXECUTION_SELECTOR;
         bytes4[] memory guardConfigBatchExecuteHandlerForSelectors = new bytes4[](1);
         guardConfigBatchExecuteHandlerForSelectors[0] = GUARD_CONFIG_BATCH_EXECUTE_SELECTOR;
+
+        bytes4[] memory executeWithPaymentHandlerForSelectors = new bytes4[](1);
+        executeWithPaymentHandlerForSelectors[0] = EXECUTE_WITH_PAYMENT_SELECTOR;
 
         // Handler selectors point to execution selectors
         bytes4[] memory guardConfigHandlerForSelectors = new bytes4[](1);
@@ -225,8 +230,8 @@ library GuardControllerDefinitions {
         schemas[6] = EngineBlox.FunctionSchema({
             functionSignature: "guardConfigBatchRequestAndApprove(((uint256,uint256,uint8,(address,address,uint256,uint256,bytes32,bytes4,bytes),bytes32,bytes,(address,uint256,address,uint256)),(uint256,uint256,address,bytes4,uint8,uint256,uint256,address),bytes32,bytes,bytes))",
             functionSelector: GUARD_CONFIG_BATCH_META_SELECTOR,
-            operationType: CONTROLLER_OPERATION,
-            operationName: "CONTROLLER_OPERATION",
+            operationType: CONTROLLER_CONFIG_OPERATION,
+            operationName: "CONTROLLER_CONFIG_OPERATION",
             supportedActionsBitmap: EngineBlox.createBitmapFromActions(metaTxRequestApproveActions),
             enforceHandlerRelations: true,
             isProtected: true,
@@ -241,14 +246,27 @@ library GuardControllerDefinitions {
         schemas[7] = EngineBlox.FunctionSchema({
             functionSignature: "executeGuardConfigBatch((uint8,bytes)[])",
             functionSelector: GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
-            operationType: CONTROLLER_OPERATION,
-            operationName: "CONTROLLER_OPERATION",
+            operationType: CONTROLLER_CONFIG_OPERATION,
+            operationName: "CONTROLLER_CONFIG_OPERATION",
             supportedActionsBitmap: EngineBlox.createBitmapFromActions(guardConfigExecutionActions),
             enforceHandlerRelations: false,
             isProtected: true,
             handlerForSelectors: guardConfigBatchExecuteHandlerForSelectors
         });
 
+        // Schema 8: GuardController.executeWithPayment (same time-delay request action as executeWithTimeLock;
+        // OWNER_ROLE grant for this selector may be added manually if the flow is enabled)
+        schemas[8] = EngineBlox.FunctionSchema({
+            functionSignature: "executeWithPayment(address,uint256,bytes4,bytes,uint256,bytes32,(address,uint256,address,uint256))",
+            functionSelector: EXECUTE_WITH_PAYMENT_SELECTOR,
+            operationType: CONTROLLER_OPERATION,
+            operationName: "CONTROLLER_OPERATION",
+            supportedActionsBitmap: EngineBlox.createBitmapFromActions(timeDelayRequestActions),
+            enforceHandlerRelations: false,
+            isProtected: true,
+            handlerForSelectors: executeWithPaymentHandlerForSelectors
+        });
+
         return schemas;
     }
 

contracts/core/lib/EngineBlox.sol

@@ -376,6 +376,16 @@ library EngineBlox {
         // Validate both execution and handler selector permissions (same as txRequest)
         _validateExecutionAndHandlerPermissions(self, msg.sender, executionSelector, handlerSelector, TxAction.EXECUTE_TIME_DELAY_REQUEST);
 
+        // Request-time validation for attached payment details.
+        // This prevents creating persistent PENDING records that later fail during
+        // `executeAttachedPayment` due to missing/zero payment fields.
+        if (paymentDetails.nativeTokenAmount > 0 || paymentDetails.erc20TokenAmount > 0) {
+            SharedValidation.validateNotZeroAddress(paymentDetails.recipient);
+        }
+        if (paymentDetails.erc20TokenAmount > 0) {
+            SharedValidation.validateNotZeroAddress(paymentDetails.erc20TokenAddress);
+        }
+
         return _txRequest(
             self,
             requester,
@@ -574,6 +584,16 @@ library EngineBlox {
         // Validate both execution and handler selector permissions
         _validateExecutionAndHandlerPermissions(self, msg.sender, metaTx.txRecord.params.executionSelector, metaTx.params.handlerSelector, TxAction.EXECUTE_META_REQUEST_AND_APPROVE);
 
+        // Request-time validation for attached payment details.
+        // `requestAndApprove` creates the request and executes via the same meta-tx flow,
+        // so we validate here to avoid persisting bad PENDING records.
+        if (metaTx.txRecord.payment.nativeTokenAmount > 0 || metaTx.txRecord.payment.erc20TokenAmount > 0) {
+            SharedValidation.validateNotZeroAddress(metaTx.txRecord.payment.recipient);
+        }
+        if (metaTx.txRecord.payment.erc20TokenAmount > 0) {
+            SharedValidation.validateNotZeroAddress(metaTx.txRecord.payment.erc20TokenAddress);
+        }
+
         TxRecord memory txRecord = _txRequest(
             self,
             metaTx.txRecord.params.requester,

contracts/core/pattern/Account.sol

@@ -2,8 +2,11 @@
 pragma solidity 0.8.34;
 
 import "../execution/GuardController.sol";
+import "../execution/interface/IGuardController.sol";
 import "../access/RuntimeRBAC.sol";
+import "../access/interface/IRuntimeRBAC.sol";
 import "../security/SecureOwnable.sol";
+import "../security/interface/ISecureOwnable.sol";
 import "../lib/utils/SharedValidation.sol";
 
 /**
@@ -51,9 +54,15 @@ abstract contract Account is GuardController, RuntimeRBAC, SecureOwnable {
 
     /**
      * @dev See {IERC165-supportsInterface}.
+     * @notice GuardController, RuntimeRBAC, and SecureOwnable each extend BaseStateMachine directly; a single
+     *         `super` chain only walks one branch. We OR the three component interface IDs here, then delegate
+     *         once to `super` for IBaseStateMachine / ERC165 — avoids tripling BaseStateMachine+ERC165 work.
      */
     function supportsInterface(bytes4 interfaceId) public view virtual override(GuardController, RuntimeRBAC, SecureOwnable) returns (bool) {
-        return GuardController.supportsInterface(interfaceId) || RuntimeRBAC.supportsInterface(interfaceId) || SecureOwnable.supportsInterface(interfaceId);
+        return interfaceId == type(IGuardController).interfaceId
+            || interfaceId == type(IRuntimeRBAC).interfaceId
+            || interfaceId == type(ISecureOwnable).interfaceId
+            || super.supportsInterface(interfaceId);
     }
 
     /**

contracts/core/security/SecureOwnable.sol

@@ -27,17 +27,20 @@ import "./interface/ISecureOwnable.sol";
  * Each operation follows a request -> approval workflow with appropriate time locks
  * and authorization checks. Operations can be cancelled within specific time windows.
  *
- * At most one ownership-transfer or broadcaster-update request may be pending at a time:
- * a pending request of either type blocks new requests until it is approved or cancelled.
+ * Pending secure requests use separate flags for ownership transfer and broadcaster update.
+ * A new ownership-transfer request is allowed if no ownership transfer is already pending
+ * (a broadcaster update may still be pending). A new broadcaster-update request is allowed only
+ * when neither type has a pending request.
  *
  * This contract focuses purely on security logic while leveraging the BaseStateMachine
  * for transaction management, meta-transactions, and state machine operations.
  */
 abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
     using SharedValidation for *;
 
-    /// @dev True while any pending ownership transfer or broadcaster update request exists; blocks new requests until handled.
-    bool private _hasOpenRequest;
+    /// @dev Tracks pending secure txs by type. Upgrading from legacy `_hasOpenRequest` / `_pendingBits` requires no pending requests.
+    bool private _hasOpenOwnershipRequest;
+    bool private _hasOpenBroadcasterRequest;
 
     /**
      * @notice Initializer to initialize SecureOwnable state
@@ -83,7 +86,7 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
      */
     function transferOwnershipRequest() public returns (uint256 txId) {
         SharedValidation.validateRecovery(getRecovery());
-        _requireNoPendingRequest();
+        _requireNoPendingRequest(SecureOwnableDefinitions.OWNERSHIP_TRANSFER);
 
         EngineBlox.TxRecord memory txRecord = _requestTransaction(
             msg.sender,
@@ -95,7 +98,7 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
             abi.encode(getRecovery())
         );
 
-        _hasOpenRequest = true;
+        _hasOpenOwnershipRequest = true;
         _logAddressPairEvent(owner(), getRecovery());
         return txRecord.txId;
     }
@@ -143,13 +146,15 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
     // Broadcaster Management
     /**
      * @dev Requests an update to the broadcaster at a specific location (index).
+     * @notice Requires no pending broadcaster-update and no pending ownership-transfer request.
      * @param newBroadcaster The new broadcaster address (zero address to revoke at location)
      * @param location The index in the broadcaster role's authorized wallets set
      * @return txId The transaction ID for the pending request (use getTransaction(txId) for full record)
      */
     function updateBroadcasterRequest(address newBroadcaster, uint256 location) public returns (uint256 txId) {
         SharedValidation.validateOwner(owner());
-        _requireNoPendingRequest();
+        _requireNoPendingRequest(SecureOwnableDefinitions.BROADCASTER_UPDATE);
+        _requireNoPendingRequest(SecureOwnableDefinitions.OWNERSHIP_TRANSFER);
 
         // Get the current broadcaster at the specified location. zero address if no broadcaster at location.
         address currentBroadcaster = location < _getSecureState().roles[EngineBlox.BROADCASTER_ROLE].walletCount
@@ -166,7 +171,7 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
             abi.encode(newBroadcaster, location)
         );
 
-        _hasOpenRequest = true;
+        _hasOpenBroadcasterRequest = true;
         _logAddressPairEvent(currentBroadcaster, newBroadcaster);
         return txRecord.txId;
     }
@@ -281,12 +286,6 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
 
     // ============ INTERNAL FUNCTIONS ============
 
-    /**
-     * @dev Reverts if an ownership-transfer or broadcaster-update request is already pending.
-     */
-    function _requireNoPendingRequest() internal view {
-        if (_hasOpenRequest) revert SharedValidation.PendingSecureRequest();
-    }
 
     /**
      * @dev Validates that the caller is the broadcaster and that the meta-tx signer is the owner.
@@ -298,25 +297,56 @@ abstract contract SecureOwnable is BaseStateMachine, ISecureOwnable {
     }
 
     /**
-     * @dev Completes ownership/broadcaster flow after approval: resets flag and returns txId.
+     * @dev Completes ownership/broadcaster flow after approval: clears the matching pending flag and returns txId.
      * @param updatedRecord The updated transaction record from approval
      * @return txId The transaction ID
      */
     function _completeApprove(EngineBlox.TxRecord memory updatedRecord) internal returns (uint256 txId) {
-        _hasOpenRequest = false;
+        _clearPendingFlagForOperation(updatedRecord.params.operationType);
         return updatedRecord.txId;
     }
 
     /**
-     * @dev Completes ownership/broadcaster flow after cancellation: resets flag, logs txId, returns txId.
+     * @dev Completes ownership/broadcaster flow after cancellation: clears the matching pending flag and returns txId.
      * @param updatedRecord The updated transaction record from cancellation
      * @return txId The transaction ID
      */
     function _completeCancel(EngineBlox.TxRecord memory updatedRecord) internal returns (uint256 txId) {
-        _hasOpenRequest = false;
+        _clearPendingFlagForOperation(updatedRecord.params.operationType);
         return updatedRecord.txId;
     }
 
+    /**
+     * @dev Reverts if the pending flag for `requestOperationType` is already set (one lane per call).
+     *      `OWNERSHIP_TRANSFER` checks only `_hasOpenOwnershipRequest` (a broadcaster update may still be pending).
+     *      `BROADCASTER_UPDATE` checks only `_hasOpenBroadcasterRequest`. Callers that need both lanes idle
+     *      (e.g. `updateBroadcasterRequest`) invoke this once per operation type.
+     * @param requestOperationType Lane to validate (`OWNERSHIP_TRANSFER` or `BROADCASTER_UPDATE`).
+     */
+    function _requireNoPendingRequest(bytes32 requestOperationType) internal view {
+        if (requestOperationType == SecureOwnableDefinitions.OWNERSHIP_TRANSFER) {
+            if (_hasOpenOwnershipRequest) revert SharedValidation.PendingSecureRequest();
+        } else if (requestOperationType == SecureOwnableDefinitions.BROADCASTER_UPDATE) {
+            if (_hasOpenBroadcasterRequest) revert SharedValidation.PendingSecureRequest();
+        } else {
+            revert();
+        }
+    }
+
+    /**
+     * @dev Clears the pending flag for a completed or cancelled secure op (approve/cancel paths).
+     * @param operationType The tx record's `operationType` (`OWNERSHIP_TRANSFER` or `BROADCASTER_UPDATE`).
+     */
+    function _clearPendingFlagForOperation(bytes32 operationType) private {
+        if (operationType == SecureOwnableDefinitions.OWNERSHIP_TRANSFER) {
+            _hasOpenOwnershipRequest = false;
+        } else if (operationType == SecureOwnableDefinitions.BROADCASTER_UPDATE) {
+            _hasOpenBroadcasterRequest = false;
+        } else {
+            revert();
+        }
+    }
+
     /**
      * @dev Transfers ownership of the contract
      * @param newOwner The new owner of the contract

docs/core/execution/lib/definitions/GuardControllerDefinitions.md

@@ -41,7 +41,7 @@ Returns predefined function schemas for GuardController execution functions
 
 Function schemas define:
 - GuardController public execution functions
-- What operation types they belong to (CONTROLLER_OPERATION)
+- What operation types they belong to (`CONTROLLER_OPERATION` for execution paths, `CONTROLLER_CONFIG_OPERATION` for guard config batch)
 - What actions are supported (time-delay request/approve/cancel, meta-tx approve/cancel/request-and-approve)
 - Whether they are protected
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "Bloxchain",
-  "version": "1.0.0-alpha.18",
+  "version": "1.0.0-alpha.19",
   "description": "Library engine for building enterprise grade decentralized permissioned applications",
   "type": "module",
   "main": "truffle-config.cjs",

package/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@bloxchain/contracts",
-  "version": "1.0.0-alpha.18",
+  "version": "1.0.0-alpha.19",
   "description": "Library engine for building enterprise grade decentralized permissioned applications",
   "files": [
     "core",

scripts/sanity-sdk/guard-controller/base-test.ts

@@ -64,7 +64,9 @@ export abstract class BaseGuardControllerTest extends BaseSDKTest {
   protected metaTxSigner: MetaTransactionSigner | null = null;
 
   // GuardController constants
-  protected readonly CONTROLLER_OPERATION_TYPE: Hex = keccak256(new TextEncoder().encode('CONTROLLER_OPERATION')) as Hex;
+  protected readonly CONTROLLER_CONFIG_OPERATION_TYPE: Hex = keccak256(
+    new TextEncoder().encode('CONTROLLER_CONFIG_OPERATION')
+  ) as Hex;
   protected readonly GUARD_CONFIG_BATCH_META_SELECTOR: Hex = keccak256(
     new TextEncoder().encode('guardConfigBatchRequestAndApprove(((uint256,uint256,uint8,(address,address,uint256,uint256,bytes32,bytes4,bytes),bytes32,bytes,(address,uint256,address,uint256)),(uint256,uint256,address,bytes4,uint8,uint256,uint256,address),bytes32,bytes,bytes))')
   ).slice(0, 10) as Hex;
@@ -407,7 +409,7 @@ export abstract class BaseGuardControllerTest extends BaseSDKTest {
       target: this.contractAddress!,
       value: BigInt(0),
       gasLimit: BigInt(1000000),
-      operationType: this.CONTROLLER_OPERATION_TYPE,
+      operationType: this.CONTROLLER_CONFIG_OPERATION_TYPE,
       executionSelector: this.GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
       executionParams: executionParams
     };
@@ -493,7 +495,7 @@ export abstract class BaseGuardControllerTest extends BaseSDKTest {
       target: this.contractAddress!,
       value: BigInt(0),
       gasLimit: BigInt(1000000),
-      operationType: this.CONTROLLER_OPERATION_TYPE,
+      operationType: this.CONTROLLER_CONFIG_OPERATION_TYPE,
       executionSelector: this.GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
       executionParams: executionParams
     };
@@ -559,13 +561,13 @@ export abstract class BaseGuardControllerTest extends BaseSDKTest {
       signerWallet.address
     );
 
-    // TxParams for controller operation (mirrors createSignedMetaTxForFunctionRegistration)
+    // TxParams for controller config operation (mirrors createSignedMetaTxForFunctionRegistration)
     const txParams = {
       requester: signerWallet.address,
       target: this.contractAddress,
       value: BigInt(0),
       gasLimit: BigInt(1_000_000),
-      operationType: this.CONTROLLER_OPERATION_TYPE,
+      operationType: this.CONTROLLER_CONFIG_OPERATION_TYPE,
       executionSelector: this.GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
       executionParams,
     };