Repository navigation
Conversation
…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>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThis 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. ChangesAttested bridge
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Merge Risk: ⚪ Minimal · up to
|
|
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. |
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
AttestedBridgeFacetruns 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):InvalidCallData), a selector the Diamond owner has not enabled withsetAttestableSelector(SelectorNotAttestable) and a selector with no facet (FunctionDoesNotExist).BridgeAttesterin the periphery registry and reverts withAttesterNotRegisteredif that address has no code. All checks run before any external call._facetCalldata, so the bridge runs exactly as on a direct call: samemsg.sender, samemsg.value, same validation and refunds, and the Diamond's shared reentrancy guard. Return data is discarded; a revert bubbles up unchanged.IAttester.atteston 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.src/Facets/AttestedBridgeFacet.solsrc/Interfaces/IAttester.solsrc/Libraries/LibAttestableSelectors.solsrc/Libraries/LibPeripheryRegistry.solPeripheryRegistryFacetstorage (com.lifi.facets.periphery_registry) from inside the Diamondtest/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.t.solLiFiDiamondwith a stand-in bridge facettest/solidity/Facets/AttestedBridgeFacet/AttestedBridgeFacet.fork.t.solAcrossFacetV4deposit through the gateway on the live Diamond; an unregistered attester reverts before dispatchtest/solidity/Libraries/LibPeripheryRegistry.fork.t.solPeripheryRegistryFacettest/solidity/utils/MockBridgeFacet.soldocs/AttestedBridgeFacet.md,docs/README.mdtasks/buildClearSigningProposal.tsstartBridgeAttestedadded toNON_USER_FACING_NAMES(see Clear signing)script/deploy/resources/buildAttestations.jsonbun attestations:mintoutputThe four sources are
^0.8.17and@custom:version 1.0.0, and build under the default profile (solc 0.8.29) andsolc_floor(0.8.17).How it composes with
CATValidatorV1_1and the attesterThe intended caller is a solver's executor contract running inside
CATValidatorV1_1.entry, the validator added in lifinance/catapultar#44:{ token: attester, amount: 1, destination: commit }.entry. The validator moves the deposit from the account to the execution target, publishespulled(token)andentryId()in transient storage, and calls the target.startBridgeAttestedon the Diamond with the bridge call and its data for the attester._facetCalldata, checks them against the order and againstpulled(token)for the running entry, and credits one receipt to the validator for that entry.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
startBridgeAttestedis permissionless. It reaches only selectors the owner enabled, on the facets registered for them, with the caller's ownmsg.senderandmsg.value, so it opens no facet call path that a direct call does not already have. What it adds is the attester call.ReentrancyError(testRevert_ReentryFromTheDispatchedFacetHitsTheSharedGuard).setAttestableSelectorsays in its NatSpec to enable only entrypoints guarded bynonReentrant._facetCalldataitself, verify anything_attesterDataclaims, and refuse to attest outside a settlement it can verify.BridgeAttestermust not be added towhitelistPeripheryFunctionsinconfig/global.json; otherwise a swap step inside the Diamond could call the attester with the Diamond asmsg.senderand no bridge behind it.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) andFOUNDRY_PROFILE=solc_floor forge build --skip 'test/**'(solc 0.8.17): both succeed. The only new warning is 5667 for_attesterData(see Open points).FOUNDRY_PROFILE=ciand with the default profile (256 fuzz runs): 35 passed.--no-storage-caching: 4 passed.bun attestations:checkpasses.verifyClearSigningsteps pass: the generator writes 72 entries,config/clearSigningProposal.jsonis unchanged, every format has a test case and none is empty.solhinton the eight changed.solfiles: 0 errors (only theimport-path-checkwarnings that remapped test imports get everywhere).prettier --checkis clean on every changed file exceptbuildAttestations.json, which fails the same way onmain.markdownlintis clean.check-spdx-headers.sh,extract-contract-version.sh(1.0.0 for all four sources), the LibAsset routing check,verify-immutable-registry.ts --strictandverify-json-duplicate-keys.tspass.bun test script/ tasks/: 7092 passed, 1 failed. The failing file,ticket-gate-placement.test.ts, is flaky onmainas well (2 of 3 reruns failed onmain, 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.jsonentry and the report forAttestedBridgeFacet,IAttester,LibAttestableSelectorsandLibPeripheryRegistry(all 1.0.0) are added to this PR after the external audit. Until thenaudit-verificationis expected to fail.Clear signing
startBridgeAttestedis added toNON_USER_FACING_NAMESintasks/buildClearSigningProposal.ts. Solver executor contracts call it during a fill and users never sign it in a wallet; itsbytesargument is an opaque facet call that an ERC-7730 descriptor cannot render. The SC team owns that list, so please confirm.setAttestableSelectorandisAttestableSelectorare covered by the existingsetandisprefixes, andconfig/clearSigningProposal.jsondoes not change.Expected bot friction
delegatecall, low-levelcalland inline assembly.securityAlertsReviewmoves 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-verificationstays red until the audit-log entry lands, per the SC feature-PR lifecycle.healthCheckInvariantsReminderwill comment because a facet is added without ahealthCheckInvariants.tschange.version-controlappends the new contract versions to the title.Open points for review
_attesterDatais named for clear signing and forwarded unread by the assembly.no-unused-varsis disabled on that one parameter; solc still prints warning 5667.Test, notTestBaseFacet: the facet has nostartBridgeTokensVia…entrypoints for the base tests. It uses a stand-in bridge facet; the real-counterparty coverage is the Across V4 fork test.config/global.jsonshould coupleAttestedBridgeFacetto theBridgeAttesterperiphery entry infacetPeripheryCouplingsis a deploy-time decision for the SC team.Checklist before requesting a review
Checklist for reviewer (DO NOT DEPLOY and contracts BEFORE CHECKING THIS!!!)