feat: added pausable to watcher,socket #213
Conversation
WalkthroughThis PR introduces pausable functionality across key entry points by implementing a new Pausable utility contract. The Watcher and Socket contracts now inherit from Pausable, with critical functions (executePayload, resolvePayload, execute, _sendPayload) guarded by whenNotPaused modifiers. Owner-controlled pause/unpause functions enable temporary halt of contract operations. Changes
Sequence DiagramsequenceDiagram
participant User
participant Contract as Socket/Watcher
participant Pausable
User->>Contract: Call execute/executePayload
Contract->>Pausable: Check whenNotPaused modifier
alt paused = true
Pausable-->>Contract: Revert with ContractPaused()
Contract-->>User: Transaction reverted
else paused = false
Pausable-->>Contract: Proceed
Contract->>Contract: Execute function logic
Contract-->>User: Return result
end
Note over Contract: Owner calls pause()
Contract->>Pausable: _pause()
Pausable->>Pausable: paused = true
Pausable->>Pausable: emit Paused()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
contracts/evmx/watcher/Watcher.sol(4 hunks)contracts/protocol/Socket.sol(4 hunks)contracts/utils/Pausable.sol(1 hunks)
| bool public paused; | ||
|
|
||
| /// @notice Event emitted when contract is paused | ||
| event Paused(); | ||
|
|
||
| /// @notice Event emitted when contract is unpaused | ||
| event Unpaused(); | ||
|
|
||
| /// @notice Modifier to check if contract is not paused | ||
| modifier whenNotPaused() { | ||
| if (paused) revert ContractPaused(); | ||
| _; | ||
| } | ||
|
|
||
| /// @notice Internal function to pause the contract | ||
| function _pause() internal { | ||
| paused = true; | ||
| emit Paused(); | ||
| } | ||
|
|
||
| /// @notice Internal function to unpause the contract | ||
| function _unpause() internal { | ||
| paused = false; | ||
| emit Unpaused(); | ||
| } |
There was a problem hiding this comment.
Fix storage layout regression before shipping.
bool public paused lives in the new base contract. When Watcher (upgradeable via Initializable) and Socket inherit this, the storage slot for paused is inserted ahead of all existing state. On upgrade, every variable defined in the derived contracts shifts down one slot: e.g. nextPayloadCount in Watcher now reads from an empty slot (zero) while the old value becomes the new paused flag (likely true), corrupting state and bricking the system. This should store the pause flag in an unstructured storage slot (or another proven-safe mechanism) so the existing layout stays untouched.
Apply this diff to keep the pause flag in a dedicated storage slot:
+library PausableStorage {
+ bytes32 internal constant STORAGE_SLOT = keccak256("socket.storage.Pausable");
+
+ struct Layout {
+ bool paused;
+ }
+
+ function layout() internal pure returns (Layout storage l) {
+ bytes32 slot = STORAGE_SLOT;
+ assembly {
+ l.slot := slot
+ }
+ }
+}
+
abstract contract Pausable {
- /// @notice Thrown when the contract is paused
error ContractPaused();
- /// @notice Paused state
- bool public paused;
+ function paused() public view returns (bool) {
+ return PausableStorage.layout().paused;
+ }
/// @notice Event emitted when contract is paused
event Paused();
/// @notice Event emitted when contract is unpaused
event Unpaused();
/// @notice Modifier to check if contract is not paused
modifier whenNotPaused() {
- if (paused) revert ContractPaused();
+ if (paused()) revert ContractPaused();
_;
}
/// @notice Internal function to pause the contract
function _pause() internal {
- paused = true;
+ PausableStorage.Layout storage store = PausableStorage.layout();
+ if (store.paused) return;
+ store.paused = true;
emit Paused();
}
/// @notice Internal function to unpause the contract
function _unpause() internal {
- paused = false;
+ PausableStorage.Layout storage store = PausableStorage.layout();
+ if (!store.paused) return;
+ store.paused = false;
emit Unpaused();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool public paused; | |
| /// @notice Event emitted when contract is paused | |
| event Paused(); | |
| /// @notice Event emitted when contract is unpaused | |
| event Unpaused(); | |
| /// @notice Modifier to check if contract is not paused | |
| modifier whenNotPaused() { | |
| if (paused) revert ContractPaused(); | |
| _; | |
| } | |
| /// @notice Internal function to pause the contract | |
| function _pause() internal { | |
| paused = true; | |
| emit Paused(); | |
| } | |
| /// @notice Internal function to unpause the contract | |
| function _unpause() internal { | |
| paused = false; | |
| emit Unpaused(); | |
| } | |
| library PausableStorage { | |
| bytes32 internal constant STORAGE_SLOT = keccak256("socket.storage.Pausable"); | |
| struct Layout { | |
| bool paused; | |
| } | |
| function layout() internal pure returns (Layout storage l) { | |
| bytes32 slot = STORAGE_SLOT; | |
| assembly { | |
| l.slot := slot | |
| } | |
| } | |
| } | |
| abstract contract Pausable { | |
| error ContractPaused(); | |
| function paused() public view returns (bool) { | |
| return PausableStorage.layout().paused; | |
| } | |
| /// @notice Event emitted when contract is paused | |
| event Paused(); | |
| /// @notice Event emitted when contract is unpaused | |
| event Unpaused(); | |
| /// @notice Modifier to check if contract is not paused | |
| modifier whenNotPaused() { | |
| if (paused()) revert ContractPaused(); | |
| _; | |
| } | |
| /// @notice Internal function to pause the contract | |
| function _pause() internal { | |
| PausableStorage.Layout storage store = PausableStorage.layout(); | |
| if (store.paused) return; | |
| store.paused = true; | |
| emit Paused(); | |
| } | |
| /// @notice Internal function to unpause the contract | |
| function _unpause() internal { | |
| PausableStorage.Layout storage store = PausableStorage.layout(); | |
| if (!store.paused) return; | |
| store.paused = false; | |
| emit Unpaused(); | |
| } | |
| } |
Summary by CodeRabbit
Release Notes