Conversation
…d EngineBlox This commit updates parameter names in the BaseStateMachine and EngineBlox libraries to enhance clarity and consistency. Specifically, it renames `roleHashReturn` to `hash` and `enforceProtectedSchemas` to `requireProtected` in the BaseStateMachine contract. In the EngineBlox library, it modifies `_newTimeLockPeriodSec` to `periodSec`, and updates function names for better readability, such as changing `createNewTxRecord` to `createTxRecord` and `addToPendingTransactionsList` to `addPendingTx`. These changes aim to improve code maintainability and understanding for future developers.
…gineBlox This commit refactors the hook management functions in the BaseStateMachine and EngineBlox libraries to improve clarity and consistency. The function names have been updated to better reflect their actions: `addTargetToFunctionHooks` is now `setHook`, `removeTargetFromFunctionHooks` is now `clearHook`, and `getFunctionHookTargets` is now `getHooks`. These changes enhance the readability of the code and ensure that the function names accurately describe their purpose, contributing to better maintainability for future developers.
…ine and EngineBlox This commit refactors the wallet update functions in the BaseStateMachine and EngineBlox libraries to improve clarity and consistency. The function names have been updated from `_updateAssignedWallet` to `_updateWallet` and `updateAssignedWallet` to `updateWallet`, better reflecting their purpose of replacing an old wallet with a new one. These changes enhance code readability and maintainability for future developers.
This commit refactors the whitelist management functions in the BaseStateMachine and GuardController contracts to improve clarity and consistency. The function names have been updated from `_addTargetToFunctionWhitelist` to `_addTargetToWhitelist` and from `_removeTargetFromFunctionWhitelist` to `_removeTargetFromWhitelist`. These changes enhance code readability and maintainability, ensuring that the function names accurately reflect their purpose in managing the whitelist of targets.
…seStateMachine and EngineBlox This commit refactors the function schema management methods in the BaseStateMachine and EngineBlox libraries to improve clarity and consistency. The method `_removeFunctionSchema` has been renamed to `_unregisterFunction`, and the corresponding comments have been updated to reflect this change. These modifications enhance code readability and maintainability, ensuring that the function names accurately describe their purpose in managing function schemas.
…aseStateMachine This commit refactors the function schema and role management methods in the BaseStateMachine contract to enhance clarity and consistency. The method `_getSecureState()` has been replaced with `_createFunctionSchema()` for creating function schemas, and `_addFunctionToRole()` is now used for adding functions to roles. These changes improve code readability and maintainability, ensuring that the function names accurately reflect their intended actions.
…StateMachine and EngineBlox This commit refactors the function schema management methods in the BaseStateMachine and EngineBlox libraries to enhance clarity and consistency. The method `_createFunctionSchema` has been renamed to `_registerFunction`, and the corresponding comments have been updated to reflect this change. Additionally, the method `_registerFunction` in the GuardController has been renamed to `_registerGuardedFunction`. These modifications improve code readability and maintainability, ensuring that the function names accurately describe their purpose in managing function schemas.
… EngineBlox This commit refactors the transaction data preparation function in the EngineBlox library by renaming `prepareTransactionData` to `buildCallData`. This change enhances clarity regarding the function's purpose, which is to construct transaction call data from execution selectors and parameters without executing the transaction. The update improves code readability and maintainability, ensuring that function names accurately reflect their intended actions.
…I files This commit updates the ABI files for multiple contracts, changing the parameter name `roleHashReturn` to `hash`. This change enhances consistency and clarity in the naming conventions used across the ABI definitions, ensuring that the parameter names accurately reflect their purpose. The update contributes to improved readability and maintainability of the codebase.
…achine and EngineBlox This commit refactors several state management functions in the BaseStateMachine and EngineBlox libraries to enhance clarity and consistency. The functions `getPendingTransactionsList`, `getWalletsInRole`, `getSupportedOperationTypesList`, `getSupportedRolesList`, and `getSupportedFunctionsList` have been renamed to `getPendingTransactions`, `getAuthorizedWallets`, `getSupportedOperationTypes`, `getSupportedRoles`, and `getSupportedFunctions`, respectively. These changes improve code readability and maintainability, ensuring that the function names accurately reflect their intended actions.
This commit refactors the ABI definitions across multiple contracts by removing the `functionSchemaExists` and `getWalletsInRole` functions, replacing them with the `getAuthorizedWallets` function. This change improves clarity in the naming conventions and ensures that the ABI accurately reflects the intended functionality. The updates contribute to better readability and maintainability of the codebase.
…chine This commit updates the function names across the RuntimeRBAC and BaseStateMachine documentation and TypeScript SDK to enhance clarity and consistency. The function `getWalletsInRole` has been renamed to `getAuthorizedWallets`, and several other functions have been streamlined to remove the "List" suffix, such as `getPendingTransactionsList` to `getPendingTransactions`, `getSupportedRolesList` to `getSupportedRoles`, and `getSupportedFunctionsList` to `getSupportedFunctions`. These changes improve code readability and maintainability, ensuring that the function names accurately reflect their intended actions.
…clarity This commit adds a validation check for zero addresses in the `addFunctionTargetHook` function of the EngineBlox library, utilizing the `SharedValidation.validateNotZeroAddress` method. Additionally, it updates the documentation in the StateAbstraction file to clarify the usage of the `_getFunctionsByOperationType` function, ensuring that it is correctly referenced in relation to the `unregisterFunction` and `_validateAnyRole` methods. These changes improve code robustness and documentation accuracy, contributing to better maintainability and understanding of the codebase.
…documentation This commit adds validation checks in the EngineBlox library to ensure that both execution and handler selectors have registered function schemas, improving the robustness of the function permission checks. Additionally, it updates the TypeScript SDK documentation to reflect the renaming of the `getWalletsInRole` function to `getAuthorizedWallets`, enhancing clarity and consistency in the API. These changes contribute to better maintainability and understanding of the codebase.
…unction unregistration This commit introduces a new entry in the Attack Vectors Codex detailing the medium severity issue of unsafe function unregistration with stale permissions. It includes a description of the attack scenario, current protections, and related tests. Additionally, the test documentation is updated to include the new `UnregisterFunctionFuzz.t.sol` file, which contains fuzz tests for this specific vulnerability, ensuring comprehensive coverage and validation of the protection mechanisms in place. The final coverage report is also updated to reflect the addition of this test, maintaining clarity and accuracy in the documentation.
…on handling This commit updates the fuzz tests in `ComprehensiveCompositeFuzz.t.sol` and `ComprehensiveStateMachineFuzz.t.sol` to improve the registration and permission handling of function schemas. It introduces new helper functions for registering and whitelisting targets, ensuring that the appropriate permissions are granted for executing functions. Additionally, the tests now include checks for insufficient balance handling in an authorized context, enhancing the robustness of the security validations. These changes contribute to better coverage and validation of the function permission mechanisms within the EngineBlox framework.
…validation This commit updates the fuzz tests in `ComprehensiveCompositeFuzz.t.sol` and `UnregisterFunctionFuzz.t.sol` to improve the validation of time-lock execution and function signature consistency. It introduces a critical assertion in the `approveTimeLockExecutionWithMetaTx` test to immediately fail if the function is executed before the designated release time, addressing potential regression issues. Additionally, it adds a check to ensure that the provided function signature matches the derived selector, preventing misleading test setups. These enhancements contribute to better coverage and robustness in the security validations of the EngineBlox framework.
…ta-tx time-lock interactions This commit revises the Attack Vectors Codex to clarify the use of meta-transactions as a privileged path for approving time-locked transactions, emphasizing the importance of proper RBAC and schema configuration to prevent bypassing time-lock protections. Additionally, it enhances the fuzz tests in `ComprehensiveCompositeFuzz.t.sol` to ensure that meta-transaction approvals are gated by permissions, verifying that unauthorized or misconfigured attempts revert correctly. The final coverage report is updated to reflect the addition of a new test, maintaining comprehensive coverage and validation of security mechanisms within the EngineBlox framework.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (55)
📝 WalkthroughWalkthroughLarge-scale API refactoring across contracts and interfaces to improve naming conventions. Renames multiple functions to remove redundant suffixes ( Changes
Sequence Diagram(s)The conditions for generating sequence diagrams are not met. While the changes introduce significant API refactoring, they are primarily renames and reorganization of existing control flows without introducing new sequential interactions between 3+ distinct components. The changes preserve existing functionality and do not alter the fundamental message-passing patterns between system actors. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
getAuthorizedWallets()to retrieve wallets for a role.getActiveRolePermissions()to query role permissions.Bug Fixes
Refactor
getWalletsInRole()togetAuthorizedWallets().roleHashReturn→hash).Tests