Skip to content

feat(AttestedBridgeFacet): add an attested gateway for bridge calls - #2583

Closed
glarov95 wants to merge 4 commits into
lifinance:mainfrom
glarov95:feat/attested-bridge-facet
Closed

glarov95 wants to merge 4 commits into
lifinance:mainfrom
glarov95:feat/attested-bridge-facet

Conversation

@glarov95

Copy link
Copy Markdown

Which Linear task belongs to this PR?

No EXSC ticket exists for this facet yet.

Why did I implement it this way?

What this adds

AttestedBridgeFacet runs an existing bridge entrypoint of the Diamond and, in the same transaction, reports the exact call to an attester contract. It is the on-chain hook for non-custodial bridge fills: a settlement contract that releases funds to a solver can rely on the attester's record that the funds left through a specific bridge call, instead of trusting the solver.

startBridgeAttested(bytes _facetCalldata, bytes _attesterData):

  1. Rejects calldata shorter than four bytes (InvalidCallData), a selector the Diamond owner has not enabled with setAttestableSelector (SelectorNotAttestable) and a selector with no facet (FunctionDoesNotExist).
  2. Reads the attester registered under BridgeAttester in the periphery registry and reverts with AttesterNotRegistered if that address has no code. All checks run before any external call.
  3. Delegatecalls the facet with _facetCalldata, so the bridge runs exactly as on a direct call: same msg.sender, same msg.value, same validation and refunds, and the Diamond's shared reentrancy guard. Return data is discarded; a revert bubbles up unchanged.
  4. Calls IAttester.attest on the attester with value 0 and this call's own calldata, only the selector swapped. attest(bytes,bytes) has the same argument encoding, so one calldata copy serves both calls, which is why the dispatch is inline assembly. A revert bubbles up unchanged and undoes the bridge, so a bridge never completes without its attestation.

This is a gateway, not a bridge facet: the bridge-facet conventions (_startBridge, startBridgeTokensVia…, validateBridgeData, refundExcessNative) stay with the dispatched facet.

File What it is
src/Facets/AttestedBridgeFacet.sol The facet
src/Interfaces/IAttester.sol The interface the attester implements
src/Libraries/LibAttestableSelectors.sol Diamond-storage set of enabled selectors; the facet's own three selectors can never be enabled
src/Libraries/LibPeripheryRegistry.sol Read-only access to the existing PeripheryRegistryFacet storage (com.lifi.facets.periphery_registry) from inside the Diamond
test/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.t.sol 35 unit tests (8 of them fuzz) on a locally deployed LiFiDiamond with a stand-in bridge facet
test/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.fork.t.sol Base fork: a native AcrossFacetV4 deposit through the gateway on the live Diamond; an unregistered attester reverts before dispatch
test/solidity/Libraries/LibPeripheryRegistry.fork.t.sol Base fork: the library reads the live registry exactly like PeripheryRegistryFacet
test/solidity/utils/MockBridgeFacet.sol Stand-in bridge facet for the unit suite
docs/AttestedBridgeFacet.md, docs/README.md Facet docs: flow, callers, the attester's checks, selector set, reentrancy, events and errors
tasks/buildClearSigningProposal.ts startBridgeAttested added to NON_USER_FACING_NAMES (see Clear signing)
script/deploy/resources/buildAttestations.json bun attestations:mint output

The four sources are ^0.8.17 and @custom:version 1.0.0, and build under the default profile (solc 0.8.29) and solc_floor (0.8.17).

How it composes with CATValidatorV1_1 and the attester

The intended caller is a solver's executor contract running inside CATValidatorV1_1.entry, the validator added in lifinance/catapultar#44:

  1. A user's Catapultar account signs a constraint whose outcome is one receipt of the attester: { token: attester, amount: 1, destination: commit }.
  2. The executor calls the validator's nine-argument entry. The validator moves the deposit from the account to the execution target, publishes pulled(token) and entryId() in transient storage, and calls the target.
  3. Inside that call the solver's executor contract calls startBridgeAttested on the Diamond with the bridge call and its data for the attester.
  4. The Diamond runs the bridge facet, then calls the attester. The attester checks that the caller is the Diamond, decodes the bridge facts from _facetCalldata, checks them against the order and against pulled(token) for the running entry, and credits one receipt to the validator for that entry.
  5. After the call, the validator's outcome check finds the receipt and forwards it to commit. Without a matching attested bridge in the same entry, the entry reverts and the account keeps its funds.

The attester is deployed separately and registered by the Diamond owner; it is not part of this PR. The two PRs do not depend on each other to compile or test.

Trust model

  • startBridgeAttested is permissionless. It reaches only selectors the owner enabled, on the facets registered for them, with the caller's own msg.sender and msg.value, so it opens no facet call path that a direct call does not already have. What it adds is the attester call.
  • The facet has no reentrancy guard of its own and relies on the Diamond's shared one: while a guarded facet runs, a re-entry through the gateway reverts with ReentrancyError (testRevert_ReentryFromTheDispatchedFacetHitsTheSharedGuard). setAttestableSelector says in its NatSpec to enable only entrypoints guarded by nonReentrant.
  • The attester's inputs are caller-controlled. The facet only guarantees that the facet call with exactly those bytes succeeded in the same transaction, called from the Diamond. An attester must therefore accept calls only from the Diamond, decode the bridge facts from _facetCalldata itself, verify anything _attesterData claims, and refuse to attest outside a settlement it can verify.
  • BridgeAttester must not be added to whitelistPeripheryFunctions in config/global.json; otherwise a swap step inside the Diamond could call the attester with the Diamond as msg.sender and no bridge behind it.
  • The owner controls the selector set and the registry entry. Disabling a selector or replacing the attester makes in-flight fills that depend on it revert until the change is undone; their funds stay in the signers' accounts.
  • The facet holds no funds and writes no Diamond storage on dispatch (test_WritesNoDiamondStorage); its only storage is the selector set.

Verification

Run locally with Foundry 1.7.1 (.foundry-version):

  • forge build (default profile, solc 0.8.29) and FOUNDRY_PROFILE=solc_floor forge build --skip 'test/**' (solc 0.8.17): both succeed. The only new warning is 5667 for _attesterData (see Open points).
  • Unit suite with FOUNDRY_PROFILE=ci and with the default profile (256 fuzz runs): 35 passed.
  • Both Base fork suites at block 51,738,000 with --no-storage-caching: 4 passed.
  • bun attestations:check passes.
  • The verifyClearSigning steps pass: the generator writes 72 entries, config/clearSigningProposal.json is unchanged, every format has a test case and none is empty.
  • solhint on the eight changed .sol files: 0 errors (only the import-path-check warnings that remapped test imports get everywhere). prettier --check is clean on every changed file except buildAttestations.json, which fails the same way on main. markdownlint is clean.
  • check-spdx-headers.sh, extract-contract-version.sh (1.0.0 for all four sources), the LibAsset routing check, verify-immutable-registry.ts --strict and verify-json-duplicate-keys.ts pass.
  • bun test script/ tasks/: 7092 passed, 1 failed. The failing file, ticket-gate-placement.test.ts, is flaky on main as well (2 of 3 reruns failed on main, 1 of 3 on this branch).

Before this PR, the gateway was reviewed internally (no critical, high or medium findings) and passed end-to-end fork and testnet runs of the full fill flow.

Audit log

The audit/auditLog.json entry and the report for AttestedBridgeFacet, IAttester, LibAttestableSelectors and LibPeripheryRegistry (all 1.0.0) are added to this PR after the external audit. Until then audit-verification is expected to fail.

Clear signing

startBridgeAttested is added to NON_USER_FACING_NAMES in tasks/buildClearSigningProposal.ts. Solver executor contracts call it during a fill and users never sign it in a wallet; its bytes argument is an opaque facet call that an ERC-7730 descriptor cannot render. The SC team owns that list, so please confirm. setAttestableSelector and isAttestableSelector are covered by the existing set and is prefixes, and config/clearSigningProposal.json does not change.

Expected bot friction

  • Olympix will likely raise alerts on the facet's delegatecall, low-level call and inline assembly. securityAlertsReview moves the PR back to draft on ready-for-review until each alert is resolved or dismissed with a reason; the reasons are in the trust model above.
  • audit-verification stays red until the audit-log entry lands, per the SC feature-PR lifecycle.
  • Once the PR is ready for review, healthCheckInvariantsReminder will comment because a facet is added without a healthCheckInvariants.ts change.
  • version-control appends the new contract versions to the title.

Open points for review

  • _attesterData is named for clear signing and forwarded unread by the assembly. no-unused-vars is disabled on that one parameter; solc still prints warning 5667.
  • The unit suite inherits Test, not TestBaseFacet: the facet has no startBridgeTokensVia… entrypoints for the base tests. It uses a stand-in bridge facet; the real-counterparty coverage is the Across V4 fork test.
  • Whether config/global.json should couple AttestedBridgeFacet to the BridgeAttester periphery entry in facetPeripheryCouplings is a deploy-time decision for the SC team.
  • The deploy and update scripts come with the deploy PR.

Checklist before requesting a review

Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)

  • I have checked that any arbitrary calls to external contracts are validated and or restricted
  • I have checked that any privileged calls (i.e. storage modifications) are validated and or restricted
  • I have ensured that any new contracts have had AT A MINIMUM 1 preliminary audit conducted on by <company/auditor>

…its libraries

startBridgeAttested delegatecalls a bridge entrypoint the Diamond owner has
enabled, then calls the attester registered under BridgeAttester in the
periphery registry with the same call bytes. LibAttestableSelectors keeps the
enabled selectors in Diamond storage, LibPeripheryRegistry reads
PeripheryRegistryFacet storage from inside the Diamond, and IAttester is the
interface the attester implements.

Signed-off-by: georgi <georgi@li.finance>
The unit suite runs on a locally deployed LiFiDiamond with a stand-in bridge
facet. It covers the dispatch checks and their order, byte-exact forwarding to
the attester, value routing, revert bubbling, the selector set and the shared
reentrancy guard. The Base fork suites send a native AcrossFacetV4 deposit
through the gateway on the live Diamond and check LibPeripheryRegistry against
the live registry.

Signed-off-by: georgi <georgi@li.finance>
Covers the dispatch flow, who calls the gateway, what an attester must check,
the selector set, reentrancy, events and errors.

Signed-off-by: georgi <georgi@li.finance>
…igning exclusion

buildAttestations.json is regenerated with bun attestations:mint.
startBridgeAttested joins NON_USER_FACING_NAMES: solver executor contracts call
it during a fill, users never sign it in a wallet, and its bytes argument is an
opaque facet call.

Signed-off-by: georgi <georgi@li.finance>
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9864a3d0-3dac-44f1-8b97-f3611ea0c787
📥 Commits

Reviewing files that changed from the base of the PR and between fcf2696 and 526286c.

📒 Files selected for processing: 12
  • docs/AttestedBridgeFacet.md
  • docs/README.md
  • script/deploy/resources/buildAttestations.json
  • src/Facets/AttestedBridgeFacet.sol
  • src/Interfaces/IAttester.sol
  • src/Libraries/LibAttestableSelectors.sol
  • src/Libraries/LibPeripheryRegistry.sol
  • tasks/buildClearSigningProposal.ts
  • test/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.fork.t.sol
  • test/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.t.sol
  • test/solidity/Libraries/LibPeripheryRegistry.fork.t.sol
  • test/solidity/utils/MockBridgeFacet.sol

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

This change adds an attested bridge facet that dispatches owner-enabled bridge selectors through the Diamond, then calls a registered attester with the original facet calldata and attester data. It also adds selector and registry libraries, tests, build metadata, and documentation.

Changes

Attested bridge

Layer / File(s) Summary
Selector permissions and registry lookup
src/Libraries/LibAttestableSelectors.sol, src/Libraries/LibPeripheryRegistry.sol, src/Facets/AttestedBridgeFacet.sol, test/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.t.sol, test/solidity/Libraries/LibPeripheryRegistry.fork.t.sol
Adds namespaced selector permissions and periphery registry reads. Selector updates require Diamond ownership and reject the gateway’s own selectors. Tests cover selector configuration, registry reads, and storage agreement.
Bridge dispatch and attester call
src/Interfaces/IAttester.sol, src/Facets/AttestedBridgeFacet.sol, tasks/buildClearSigningProposal.ts, test/solidity/Facets/AttestedBridgeFacet/*, test/solidity/utils/MockBridgeFacet.sol
Adds the attester interface and gateway dispatch. The facet checks calldata, selector enablement, Diamond facet availability, and attester code before dispatch. It bubbles call failures, discards successful return data, and forwards no value to the attester. Unit, fuzz, and fork tests cover the call flow and its failure cases.
Deployment record and usage documentation
script/deploy/resources/buildAttestations.json, docs/AttestedBridgeFacet.md, docs/README.md
Adds the facet’s build attestation and documents its call flow, selector configuration, attester requirements, and deployment setup. Adds the new document to the Facets index.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 52628

No identified issue blocks merging after normal build and test checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: adding an attested gateway for bridge calls.
Description check Passed The description is detailed and covers the implementation, rationale, tests, security model, documentation, and known open points. It also identifies the pending external audit and leaves reviewer-own…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Description check

Explanation

The description is detailed and covers the implementation, rationale, tests, security model, documentation, and known open points. It also identifies the pending external audit and leaves reviewer-owned checklist items unchecked.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (11 skipped: 11 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

@gvladika

gvladika commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Superseded by #2616, which carries these four commits unchanged (authorship kept) from a branch in lifinance/contracts, since a fork PR can't be merged here. Follow-up review and fixes continue there. Thanks for the work on this.

@gvladika gvladika closed this Nov 3, 2026
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.

2 participants