Make ERC7786OpenBridge tolerant to failing gateways#251
Open
Amxx wants to merge 2 commits into
Open
Conversation
Wrap each gateway sendMessage in try/catch so a single failing gateway does not block the others, and require at least the threshold number of successful sends. Also reject non-contract gateways in _addGateway.
Emit ERC7786OpenBridgeSendMessageFailed when a gateway's sendMessage reverts, and add a test covering delivery through a healthy gateway when another one reverts and the threshold is still met.
WalkthroughOpenBridge now tolerates individual gateway send failures, emits failure events, enforces successful gateway thresholds, rejects non-contract gateways, and adds mock controls and tests for resilient delivery. ChangesGateway resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Sender
participant OpenBridge
participant FailingGateway
participant WorkingGateway
participant Destination
Sender->>OpenBridge: sendMessage
OpenBridge->>FailingGateway: sendMessage
FailingGateway-->>OpenBridge: revert
OpenBridge-->>Sender: failure event
OpenBridge->>WorkingGateway: sendMessage
WorkingGateway->>Destination: deliver message
WorkingGateway-->>OpenBridge: message ID
OpenBridge-->>Sender: successful delivery events
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
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.
Fixes https://audits.openzeppelin.com/openzeppelin-solidity/project/openzeppelin-community-contracts/e2bb51d8-df7a-4d26-934a-be65c637d4e5/issue/any-single-gateway-revert-blocks-sendmessage-for-all-gateways
Summary
Makes
ERC7786OpenBridgeresilient to individual gateway failures on the source side, so a single misbehaving gateway can no longer block a message that can still satisfy the N-of-M threshold.Changes
sendMessage: eachgateway.sendMessageis now wrapped intry/catch. A reverting gateway no longer aborts the whole send; instead the loop continues with the others. After the loop, the call requires at leastgetThreshold()successful sends (ERC7786OpenBridgeInsufficientGatewaysotherwise).ERC7786OpenBridgeSendMessageFailed(address indexed gateway): new event emitted for each gateway whosesendMessagereverts, for observability._addGateway: rejects non-contract addresses (ERC7786OpenBridgeGatewayNotAContract), covering both the constructor andaddGateway.viaIR).Tests
gateway resiliencetest: two synchronous gateways, threshold 1, one gateway set to revert on send — asserts the message is still delivered through the healthy gateway and thatERC7786OpenBridgeSendMessageFailedis emitted for the failing one.bad return value/EOArecipient cases: under the synchronous mock, the destination revert is now caught on the send side, so they revert withERC7786OpenBridgeInsufficientGateways.ERC7786GatewayMockgains asetRevertOnSenttoggle to drive the resilience test.All tests pass; prettier / eslint / solhint clean.
Summary by CodeRabbit