-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add test scaffolding for cross-chain tests #1121
Merged
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
a60a45a
to
424fb7d
Compare
3ceda73
to
a880743
Compare
a880743
to
5e4f841
Compare
kronosapiens
reviewed
Mar 6, 2023
be61fcd
to
91e9996
Compare
91e9996
to
3ca3c14
Compare
kronosapiens
previously approved these changes
Mar 9, 2023
kronosapiens
previously approved these changes
Mar 10, 2023
test/cross-chain/cross-chain.js
Outdated
const ethersHomeSigner = new ethers.providers.JsonRpcProvider(homeRpcUrl).getSigner(); | ||
|
||
before(async () => { | ||
await exec( |
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.
Couldn't this be an invocation of start-bridging-environment.sh
with different arguments?
4969e32
to
cfd22d0
Compare
cfd22d0
to
2924d86
Compare
kronosapiens
approved these changes
Mar 13, 2023
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.
The original version of this branch was a very quick-and-dirty version to get the dev environment working for those working on cross-chain safes. I've tried to clean it up and get it in to a state that can be used for the cross-chain reputation work that's cooking.
One of the functional additions here is to do with coverage - we will want colonies on both sides being tested, so all tests related to cross-chain are run twice. Once with
solidity-coverage
covering the home chain, and once withsolidity-coverage
covering the foreign chain. This seems to work based on my limited testing, but will only become clear in a build like this once there is functionality specific to one side or the other in the colony contracts.In order for this to work, tests to do with cross-chain behaviour need to be written in a way that is agnostic to which chain
truffle
is covering, hence the use ofethers
throughout the example bridging test. I can't see a way to not do that.Another practical addition is a toy bridge monitor, which is started with such tests and makes sure bridged transactions go each way. The monitor, and indeed the bridged contracts, are not suitable for production, as there are no permissions. They are modelled on the AMBs set up on Gnosis but they could be anything, in principle.