feat: add watched L1 subnet contract to /v2/info#237
Conversation
Codecov Report
@@ Coverage Diff @@
## master #237 +/- ##
==========================================
+ Coverage 91.19% 92.26% +1.06%
==========================================
Files 6 6
Lines 284 336 +52
==========================================
+ Hits 259 310 +51
- Misses 25 26 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
b09285e to
95f1213
Compare
|
Also add |
|
I'll look into these failures today. |
kantai
left a comment
There was a problem hiding this comment.
These changes look good to me, but the register changes need some end-to-end testing -- I think the "happy path" is tested with the existing deposit + withdraw l1_observe_tests, but we should also test the "unhappy path" where a withdrawal is attempted from a non-registered contract.
Thoughts on where that test should go? I could put it in l1_observer_test.rs, because it will be similar to those tests, but it won't be testing the l1 observer. Is there an existing good place for it? |
|
4e6c297 to
9d1d8cc
Compare
Adds this field to the response: ``` "watch_contract": "ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM.subnet" ```
Track the asset registration from the L1 (`register-new-(n)ft-contract`) and trigger a corresponding call in the L2 subnet boot contract. This call also triggers an L2 event for observers. Additionally, the registered contracts are recorded in the contract, allowing calls to `(n)ft-withdraw?` to error out if the asset is not registered.
If a user tries to withdraw an unregistered asset from the L2, the withdrawal should fail and they should retain ownership.
d60260d to
0113342
Compare
|
The last commit looks unrelated, but it is just cleaning up a bunch of warnings that were annoying during the build/test process. |
e84d4c3 to
cd89d2c
Compare
cd89d2c to
48a1617
Compare
bbda511 to
eb43849
Compare
kantai
left a comment
There was a problem hiding this comment.
LGTM!
You should make sure the new test module gets picked up by the test matrix step in github CI, though.
In .github/workflows/ci.yml, on step compute-layer-1-tests, it currently does:
cargo test --workspace --bin=subnet-node -- l1_ --list --format=terse | sed -e 's/: test//g' | jq -ncR '{"test-name": [inputs]}' > test_names.jsonto create a JSON struct listing the tests, but the cargo test filter l1_ is too restrictive for this. I think just changing that to l would work:
cargo test --workspace --bin=subnet-node -- l --list --format=terse | sed -e 's/: test//g' | jq -ncR '{"test-name": [inputs]}' > test_names.jsonAlso refactored a bit to reduce code duplication.
This ensures that the l2_withdrawal test gets picked up as well.
1f64a80 to
77c6625
Compare
Adds this field to the response: