Partial Fast Bridge #36
Merged
Conversation
This file contains 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
shalzz
force-pushed
the
feature/fast-bridge
branch
from
fae6471
to
dfe5ee8
Compare
- sendFast() doesn't need to call the SafeBridge, we can let an incentivized party do it. - Moved the call to the SafeBridge to a separate sendSafe() function, payable in case any bridging fee is needed. - ForeignGateway.relayRuling() may be called twice (by both the safe and fast bridges) without issue because we mark the ruling as relayed the first time and check it.
contract SafeArbitrumBridge --> SafeBridgeArbitrum contract SafeGnosisBridge --> SafeBridgeGnosis function sendCrossDomainMessage() --> sendSafe() event DisputeHash --> OutgoingDispute event OutboxMessage --> OutgoingMessage event ClaimMade --> ClaimReceived parameter messagePreImage --> message
|
Code Climate has analyzed commit 4adff44 and detected 0 issues on this pull request. View more on Code Climate. |
jaybuidl
approved these changes
Feb 8, 2022
This was referenced Feb 8, 2022
shalzz
commented
Feb 8, 2022
| // The safe bridge sends the encoded data to the FastBridgeReceiver | ||
| // in order for the FastBridgeReceiver to resolve any potential | ||
| // challenges and then forwards the message to the actual | ||
| // intended recipient encoded in `data` | ||
| // TODO: For this encodedData needs to be wrapped into an | ||
| // IFastBridgeReceiver function. | ||
| // TODO: add access checks for this on the FastBridgeReceiver. | ||
| safebridge.sendCrossDomainMessage(address(fastBridgeReceiver), encodedData); | ||
| // TODO: how much ETH should be provided for bridging? add an ISafeBridge.bridgingCost() |
There was a problem hiding this comment.
Bridging from L2 to L1 does not need any bridging cost/fees.
There was a problem hiding this comment.
Right. Given the existing native bridges, the fees would be required only for L1 to L2, and we are not using the SafeBridge for it right now. So it's more for future proofing.
| * @param _receiver The L1 contract address who will receive the calldata | ||
| * @param _calldata The receiving domain encoded message data. | ||
| */ | ||
| function sendFast(address _receiver, bytes memory _calldata) external { | ||
| require(msg.sender == fastSender, "Access not allowed: Fast Sender only."); |
There was a problem hiding this comment.
The reason I didn't add this initially was to make the fast bridge app-agnostic.
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.
This still needs access control around the fast bridge, but It shouldn't be a blocker for the demo.