feat(contracts): add reentrancy guards and harden Bridge security by WillPapper · Pull Request #89 · SyndicateProtocol/synddb

WillPapper · 2026-02-19T21:45:59Z

Summary

Switched to ReentrancyGuardTransient (EIP-1153 transient storage) for cheaper gas vs deprecated storage-based ReentrancyGuard
Added nonReentrant modifier to initializeMessage, batchInitializeMessage, initializeAndHandleMessage, and wrapNativeToken to prevent cross-message reentrancy during target contract callbacks
Refactored initializeAndHandleMessage to use internal function variants (_initializeMessage, _handleMessageInternal) to avoid nested nonReentrant guard failures
Added whenNotPaused modifier to state-changing functions via OpenZeppelin Pausable
Enhanced sequencer signature verification with chainId, nonce, and deadline for replay protection
Optimized storage by storing payloadHash (bytes32) instead of full payload (bytes) in MessageState
Updated all test files for new function signatures and signature format

Test plan

All 192 tests pass locally (forge test)
Formatting passes (forge fmt)
CI workflows pass

🤖 Generated with Claude Code

- Switch to ReentrancyGuardTransient (EIP-1153) for cheaper gas - Add nonReentrant to initializeMessage, batchInitializeMessage, initializeAndHandleMessage, and wrapNativeToken - Refactor initializeAndHandleMessage to use internal variants (_initializeMessage, _handleMessageInternal) to avoid nested nonReentrant calls - Add Pausable modifier (whenNotPaused) to state-changing functions - Include chainId, nonce, and deadline in sequencer signature hash for replay protection - Store payloadHash instead of full payload in MessageState for storage optimization - Add deadline support for message expiration - Update all test files for new function signatures and signature format Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This pull request enhances the Bridge contract's security by adding reentrancy protection, pausability, replay attack prevention, and several other hardening measures. The changes represent a significant security upgrade but also introduce breaking changes to the contract's storage layout and API.

Changes:

Added ReentrancyGuardTransient (EIP-1153) and Pausable modifiers to protect against reentrancy attacks and enable emergency stops
Enhanced sequencer signature verification with chainId, nonce, and deadline for comprehensive replay protection
Optimized storage by replacing full payload with payloadHash in MessageState struct
Added message expiration via deadline parameter and owner-controlled message cancellation
Implemented configurable gas limits for external calls to prevent griefing attacks

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
contracts/src/Bridge.sol	Core security enhancements: added reentrancy guards, pausability, nonce-based replay protection, gas limits, and message cancellation. Refactored message handling to use internal functions.
contracts/src/interfaces/IBridge.sol	Updated interface signatures to include deadline parameter and payload parameter for handleMessage. Added cancelMessage function.
contracts/src/types/DataTypes.sol	Modified MessageState struct to store payloadHash instead of payload and added deadline field (breaking storage change).
contracts/test/BridgeTest.t.sol	Updated all test cases for new function signatures, added signature creation helpers with nonce and deadline support, added partial failure test for batch operations.
contracts/test/use-cases/base/UseCaseBaseTest.sol	Updated signature helper to include bridge parameter, chainId, nonce, and deadline in signature creation.
contracts/test/use-cases/*.t.sol	Updated all use case tests to pass bridge instance to signature helper and provide payload to handleMessage calls.
contracts/test/ModuleAddingAndRemovingTest.t.sol	Updated signature creation and message handling calls for new API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T21:57:42Z

contracts/src/Bridge.sol

+        // Check message expiration
+        if (state.deadline != 0 && block.timestamp > state.deadline) {
+            revert MessageExpired(messageId, state.deadline);
+        }


Missing test coverage for message deadline expiration. There should be tests verifying that:

Messages with a deadline can be executed before the deadline expires

Messages with deadline = 0 can be executed at any time (no expiration)

Messages fail with MessageExpired error when executed after the deadline

Edge case: messages executed exactly at the deadline timestamp

Copilot · 2026-02-19T21:57:43Z

contracts/src/Bridge.sol

+    function cancelMessage(bytes32 messageId) external onlyOwner {
+        MessageState storage state = messageStates[messageId];
+
+        if (state.stage == ProcessingStage.NotStarted) {
+            revert MessageNotInitialized(messageId);
+        }
+
+        if (state.stage != ProcessingStage.PreExecution) {
+            revert MessageNotCancellable(messageId, state.stage);
+        }
+
+        state.stage = ProcessingStage.Rejected;
+
+        emit MessageCancelled(messageId);
    }


Missing test coverage for the new cancelMessage function. There should be tests verifying that:

Owner can cancel a message in PreExecution stage

Cannot cancel a message that hasn't been initialized (NotStarted stage)

Cannot cancel a message that's already executing, completed, or rejected

Non-owner cannot cancel messages

MessageCancelled event is emitted correctly

Copilot · 2026-02-19T21:57:43Z

contracts/src/Bridge.sol

+    /**
+     * @notice Pauses the bridge, preventing message initialization and handling
+     * @dev Only callable by owner
+     */
+    function pause() external onlyOwner {
+        _pause();
+    }
+
+    /**
+     * @notice Unpauses the bridge, allowing normal operations
+     * @dev Only callable by owner
+     */
+    function unpause() external onlyOwner {
+        _unpause();
+    }


Missing test coverage for the new Pausable functionality. There should be tests verifying that:

Owner can pause the bridge

Owner can unpause the bridge

Non-owner cannot pause/unpause

When paused, initializeMessage reverts

When paused, batchInitializeMessage reverts

When paused, handleMessage reverts

When paused, batchHandleMessage reverts

When paused, initializeAndHandleMessage reverts

When paused, wrapNativeToken reverts

Normal operations work after unpausing

Copilot · 2026-02-19T21:57:43Z

contracts/src/Bridge.sol

+        // External call with gas limit to prevent griefing
+        (bool success, bytes memory returnData) =
+            state.targetAddress.call{value: state.nativeTokenAmount, gas: messageCallGasLimit}(payload);


Missing test coverage for the new setMessageCallGasLimit function and gas limit enforcement. There should be tests verifying that:

Owner can set a new gas limit (≥ 100,000)

Setting a gas limit below 100,000 reverts with GasLimitTooLow

Non-owner cannot set the gas limit

MessageCallGasLimitUpdated event is emitted correctly

Message execution respects the gas limit (target contracts receive only the specified gas)

Messages that exceed the gas limit fail gracefully

Copilot · 2026-02-19T21:57:43Z

contracts/src/Bridge.sol

 import {ProcessingStage, MessageState, SequencerSignature, KeyType} from "src/types/DataTypes.sol";
 import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";
+import {ReentrancyGuardTransient} from "@openzeppelin/contracts/utils/ReentrancyGuardTransient.sol";


Potential compatibility issue: ReentrancyGuardTransient uses EIP-1153 transient storage (tstore/tload opcodes) which requires Solidity 0.8.24 or later and the Cancun network upgrade. The contract uses Solidity 0.8.30, which should support this, but deployment will fail on networks that haven't activated the Cancun upgrade. Consider documenting the network requirements or adding a compatibility check in deployment scripts.

Copilot · 2026-02-19T21:57:43Z

contracts/src/types/DataTypes.sol

 struct MessageState {
    bytes32 messageId; // Unique identifier for the message
    address targetAddress; // Address that will receive the message call
    ProcessingStage stage; // Current processing stage
-    bytes payload; // Encoded function call data
+    bytes32 payloadHash; // Hash of encoded function call data (storage optimization)
    uint256 createdAt; // Block timestamp when message was created
+    uint256 deadline; // Timestamp after which message cannot be executed (0 = no deadline)
    uint256 nativeTokenAmount; // Amount of native token to transfer with call
 }


Breaking storage layout change: The MessageState struct has been modified, changing bytes payload to bytes32 payloadHash and adding uint256 deadline. This breaks storage compatibility with any previously deployed version of the contract. If this contract is meant to be upgradeable or if there are existing deployments, this change will cause data corruption. Consider:

If this is a new deployment, document that this version is incompatible with previous versions

If upgradeability is needed, a migration path or new contract version must be used

Add a version identifier to help track which version is deployed

Copilot · 2026-02-19T21:57:44Z

contracts/src/Bridge.sol

+                payloadHash,
+                nativeTokenAmount,
+                deadline,
+                sequencerNonces[msg.sender]


Critical security bug: Using sequencerNonces[msg.sender] in signature verification is incorrect. msg.sender here is the message initializer (the caller of initializeMessage), not the sequencer who signed the message. This breaks replay protection if the message initializer and sequencer are different addresses.

The nonce should track the signer's nonce, not the caller's nonce. The signature verification should use the nonce before it's known who the signer is (chicken-and-egg problem), or the nonce should be passed as a parameter.

Recommended fix: Pass the expected nonce as a parameter to _verifySequencerSignature, retrieve it from sequencerNonces[signer] after recovery, and verify they match. The signature should be created with the signer's current nonce.

Copilot AI review requested due to automatic review settings February 19, 2026 21:46

Copilot started reviewing on behalf of WillPapper February 19, 2026 21:46 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(contracts): add reentrancy guards and harden Bridge security#89

feat(contracts): add reentrancy guards and harden Bridge security#89
WillPapper wants to merge 1 commit intomainfrom
wpapper/contract-updates

WillPapper commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

WillPapper commented Feb 19, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants