Bridge test env #17941
Merged
Bridge test env #17941
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
dariorussi
requested review from
patrickkuo,
longbowlu and
Bridgerz
and removed request for
ronny-mysten
longbowlu
reviewed
May 31, 2024
| scenario.next_tx(@0xAAAA); | ||
| let mut bridge = scenario.take_shared<Bridge>(); | ||
| bridge.send_token(target_chain_id, eth_address, coin, scenario.ctx()); | ||
| test_scenario::return_shared(bridge); |
There was a problem hiding this comment.
we can check a few things here:
- assert the burn happened
- token_transfer_records has a new entry with correct metadata
| scenario.next_tx(sender); | ||
| let mut bridge = scenario.take_shared<Bridge>(); | ||
| bridge.register_foreign_token<T>(treasury_cap, upgrade_cap, &metadata); | ||
| test_scenario::return_shared(bridge); |
There was a problem hiding this comment.
check waiting_room and treasuries have proper changes here
| // scenario.end(); | ||
| // } | ||
| #[test] | ||
| fun test_register_foreign_token() { |
There was a problem hiding this comment.
-
do we have a test case for
coin::total_supply(&tc) != 0?
https://github.com/MystenLabs/sui/blob/main/crates/sui-framework/packages/bridge/sources/treasury.move#L103 -
do we have some test cases for
add_new_token?
There was a problem hiding this comment.
test_register_foreign_token_non_zero_supplyadd_default_tokensto set up the default token does that for the happy path every time you set up the bridge andtest_add_token_price_zero_value,test_add_token_malformed_1,test_add_token_malformed_2,test_add_token_malformed_3andtest_add_native_token_noptest the not so happy path
dariorussi
force-pushed
the
bridge_test_env
branch
from
83cb9b3
to
bb1c5da
Compare
|
thanks @longbowlu for the review and added the |
dariorussi
force-pushed
the
bridge_test_env
branch
from
bb1c5da
to
8efb2cf
Compare
dariorussi
force-pushed
the
bridge_test_env
branch
from
8efb2cf
to
3517eb3
Compare
tx-tomcat
pushed a commit
to tx-tomcat/sui-network
that referenced
this pull request
Jul 29, 2024
## Description Increase test coverage and create a test environment for the bridge that should make it easier to test. At this point once we have API to sign messages we can reach 100% coverage. All missing tests are related to signature generation, function depending on that, or versioning. API to sign messages and create pub/priv keys are coming that would allow us to create a great testing environment and experience. Current coverage is as follows ``` +-------------------------+ | Move Coverage Summary | +-------------------------+ Module 000000000000000000000000000000000000000000000000000000000000000b::treasury >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::message_types >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::chain_ids >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::message >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::limiter >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::crypto >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::committee >>> % Module coverage: 100.00 Module 000000000000000000000000000000000000000000000000000000000000000b::bridge >>> % Module coverage: 51.21 +-------------------------+ | % Move Coverage: 82.32 | +-------------------------+ ``` The test environment needs more work but it will come in time. For now this work was mostly to reach a good test coverage. It's important to check this in for 2 main reasons: it increases test coverage significantly and allows others to work on it and improve it. ## Test plan This is it --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
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.
Description
Increase test coverage and create a test environment for the bridge that should make it easier to test.
At this point once we have API to sign messages we can reach 100% coverage. All missing tests are related to signature generation, function depending on that, or versioning. API to sign messages and create pub/priv keys are coming that would allow us to create a great testing environment and experience.
Current coverage is as follows
The test environment needs more work but it will come in time. For now this work was mostly to reach a good test coverage.
It's important to check this in for 2 main reasons: it increases test coverage significantly and allows others to work on it and improve it.
Test plan
This is it
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.