-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarify cross-chain applications as examples #194
Conversation
Clarify cross-chain applications as examples
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there are some formatting changes here, which are good - but it would be helpful if we had some way to enforce uniform formatting as we do now with our .go
files. I think we can use forge fmt
, we would just have to add the formatting rules to foundry.toml
. We could add forge fmt
to abi bindings script (and probably rename it) and then also check it in ci.
contracts/src/CrossChainApplications/examples/ERC20Bridge/ERC20Bridge.sol
Outdated
Show resolved
Hide resolved
In each of the example contract |
Deferring this to a new issue #198. |
@@ -2,8 +2,9 @@ | |||
|
|||
This directory includes cross-chain applications that are built on top of the [Teleporter protocol](../Teleporter/README.md). | |||
|
|||
## Applications | |||
## Example Applications |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add an additional note here that all contracts under /examples
are included solely for demonstration purpose and not intended for production use?
Probably also in GETTING_STARTED.md
. It will be slight overkill, but I'd definitely prefer to be on the "over communicated point" side here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two tiny nits, and I agree with Michael's comment about adding comments wherever we can indicating the contracts are not production worthy.
import {TeleporterRegistry} from "../../../Teleporter/upgrades/TeleporterRegistry.sol"; | ||
import {UnitTestMockERC20} from "../../../Mocks/UnitTestMockERC20.sol"; | ||
import {TeleporterRegistry} from "@teleporter/upgrades/TeleporterRegistry.sol"; | ||
import {UnitTestMockERC20} from "../../../../Mocks/UnitTestMockERC20.sol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also add a remapping for the Mocks
directory?
contracts/src/CrossChainApplications/examples/ExampleMessenger/README.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Michael Kaplan <[email protected]> Signed-off-by: minghinmatthewlam <[email protected]>
Co-authored-by: Michael Kaplan <[email protected]> Signed-off-by: minghinmatthewlam <[email protected]>
…/README.md Co-authored-by: cam-schultz <[email protected]> Signed-off-by: minghinmatthewlam <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM/. One question about vm.stopPrank
in the tests.
Why this should be merged
Fixes #134
How this works
@teleporter
in remappings, and used for cross-chain appsHow this was tested
CI
How is this documented
READMEs