Skip to content

Record all gateways in ERC7786OpenBridge outbox#252

Open
Amxx wants to merge 2 commits into
masterfrom
erc7786-bridge-outbox-tracking
Open

Record all gateways in ERC7786OpenBridge outbox#252
Amxx wants to merge 2 commits into
masterfrom
erc7786-bridge-outbox-tracking

Conversation

@Amxx

@Amxx Amxx commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Inspired by https://audits.openzeppelin.com/openzeppelin-solidity/project/openzeppelin-community-contracts/e2bb51d8-df7a-4d26-934a-be65c637d4e5/issue/sendid-and-outboxdetails-omit-gateways-that-return-bytes32-0

Summary

In ERC7786OpenBridge.sendMessage, the outbox is now populated with an entry for every gateway the message is posted to, instead of only the gateways that return a non-zero id.

Previously, a gateway returning a zero id left its outbox slot as the default Outbox(address(0), 0), producing gaps in the array emitted via OutboxDetails and hashed into sendId. Now each slot holds Outbox(gateway, id) regardless of the returned id, so OutboxDetails and the derived sendId faithfully reflect the complete set of gateways. needsId is unchanged in meaning: a sendId is still only computed/emitted when at least one gateway returns a non-zero id.

Tests

  • Reworked the test fixture to use ERC7786GatewayMock directly (synchronous delivery), removing the Axelar dependency from these tests.
  • ERC7786GatewayMock gains a configurable sendId (setSendId, default 0) so the non-zero-id path can be exercised.
  • New outbox tracking tests:
    • a gateway returning a non-zero id causes OutboxDetails to be emitted with an outbox listing every gateway (including zero-id ones with their address), and a matching sendId;
    • when all gateways return zero, OutboxDetails is not emitted and sendId stays zero.

All tests pass; prettier / eslint / solhint clean.

Summary by CodeRabbit

  • Bug Fixes

    • Improved cross-chain message tracking when gateways return a mix of assigned and zero message IDs.
    • Ensured outbox details consistently include all participating gateways.
    • Preserved expected behavior when all gateways return zero IDs.
  • Tests

    • Added coverage for outbox tracking and gateway-specific message events.

Fill the outbox with an entry for every gateway (including those that
return a zero id) instead of leaving gaps, so OutboxDetails and the
derived sendId reflect the complete set of gateways the message was
posted to.
@Amxx
Amxx requested a review from a team as a code owner July 17, 2026 13:55
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1c8c1a8a-da0b-43b8-b9e3-9d899e2ce202

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

ERC7786OpenBridge now includes every gateway response in outbox tracking, while the gateway mock supports configurable send IDs. Tests update fixture wiring and cover non-zero and zero-ID behavior.

Changes

OpenBridge outbox tracking

Layer / File(s) Summary
Configurable gateway responses and fixtures
contracts/mocks/crosschain/ERC7786GatewayMock.sol, test/crosschain/ERC7786OpenBridge.test.js
The gateway mock accepts configurable send IDs, and tests use local-chain protocol endpoints with updated gateway and remote-bridge assertions.
Per-gateway outbox population
contracts/crosschain/ERC7786OpenBridge.sol
Each gateway address and returned ID is recorded in the outbox, while needsId tracks whether any returned ID is non-zero.
Outbox and message event assertions
test/crosschain/ERC7786OpenBridge.test.js
Tests validate per-gateway MessageSent events and OutboxDetails behavior for non-zero and all-zero send IDs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A bunny hops through gateways bright,
Recording each ID just right.
Zero or non-zero, all entries stay,
Tests chase the outbox trail today.
“Carrots!” cheers the bridge—“hooray!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: recording every gateway in the ERC7786OpenBridge outbox.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch erc7786-bridge-outbox-tracking

Comment @coderabbitai help to get the list of available commands.

@Amxx
Amxx requested a review from ernestognw July 17, 2026 13:56
@Amxx Amxx added this to the audit milestone Jul 17, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/crosschain/ERC7786OpenBridge.test.js`:
- Around line 175-176: Await both state-changing setSendId calls in the test
setup before invoking sendMessage, ensuring the gateway-ID configuration
transactions for this.protocoles[0] and this.protocoles[2] are mined and
observable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77cae731-36f9-4420-bc8a-cc5cc8ef4b26

📥 Commits

Reviewing files that changed from the base of the PR and between f7e5f08 and ba60d21.

📒 Files selected for processing (3)
  • contracts/crosschain/ERC7786OpenBridge.sol
  • contracts/mocks/crosschain/ERC7786GatewayMock.sol
  • test/crosschain/ERC7786OpenBridge.test.js

Comment thread test/crosschain/ERC7786OpenBridge.test.js Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant