Bridge via Hinting #103
Merged
Bridge via Hinting #103
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
hayesgm
force-pushed
the
hayesgm/across-first
branch
2 times, most recently
from
34ac678 to
861e453
Compare
hayesgm
force-pushed
the
hayesgm/across-first
branch
from
781dbd8 to
995e88f
Compare
kevincheng96
approved these changes
Nov 15, 2024
src/builder/actions/Actions.sol
Outdated
| @@ -1,6 +1,8 @@ | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| pragma solidity ^0.8.27; | |||
|
|
|||
| import {console} from "../console.sol"; | |||
There was a problem hiding this comment.
nit: we've been preferring absolute paths over relative paths for imports
src/builder/actions/Actions.sol
Outdated
| @@ -1,6 +1,8 @@ | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| pragma solidity ^0.8.27; | |||
|
|
|||
| import {console} from "../console.sol"; | |||
There was a problem hiding this comment.
Can't you just import it from Foundry directly? via import {console} from "forge-std/console.sol"
There was a problem hiding this comment.
Yes, I could, but then I get a stack too deep error. Go figure :(
| @@ -424,6 +455,163 @@ contract QuarkBuilderTransferTest is Test, QuarkBuilderTest { | |||
| assertNotEq(result.eip712Data.hashStruct, hex"", "non-empty hashStruct"); | |||
| } | |||
|
|
|||
| function testSimpleBridgeWithAcrossTransferSucceeds() public { | |||
There was a problem hiding this comment.
I think a better place to put this would be in BridgeLogic.t.sol so we have a unified place to find the bridging tests.
This patch simply reverses the order s.t. we bridge via Across if possible, then CCTP otherwise. Technically this means we'll never bridge via CCTP, but we'll probably adjust our overall system to allow user-hinting, etc, to retain CCTP support.
Also, we move first-class support for console logs. Note: (a) we'll need to implement console logs in Eth.swift, and (b) we should just update `forge-std` and then use `import {console} from "forge-std/console.sol";` but upgrading forge-std causes stack too deep issues.
hayesgm
force-pushed
the
hayesgm/across-first
branch
from
c8e6569 to
713238f
Compare
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 patch simply reverses the order s.t. we bridge via Across if possible, then CCTP otherwise based on a user hint (
preferAcross).Also, we move first-class support for console logs. Note: (a) we'll need to implement console logs in Eth.swift, and (b) we should just update
forge-stdand then useimport {console} from "forge-std/console.sol";but upgrading forge-std causes stack too deep issues.