hirosystems / stacks-subnets

Stacks Subnets: a layer-2 scaling solution for Stacks, intended for high-throughput, low-latency workloads
http://docs.hiro.so
GNU General Public License v3.0
51 stars 13 forks source link

feat: add watched L1 subnet contract to /v2/info #237

Closed obycode closed 1 year ago

obycode commented 1 year ago

Adds this field to the response:

"watch_contract": "ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM.subnet"
codecov-commenter commented 1 year ago

Codecov Report

Merging #237 (eb43849) into master (54c653c) will increase coverage by 1.06%. The diff coverage is n/a.

:exclamation: Current head eb43849 differs from pull request most recent head 77c6625. Consider uploading reports for the commit 77c6625 to get more accurate results

@@            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     
Impacted Files Coverage Δ
core-contracts/contracts/helper/simple-ft.clar 75.00% <ø> (-1.48%) :arrow_down:
...contracts/contracts/helper/simple-nft-no-mint.clar 66.66% <ø> (ø)
core-contracts/contracts/helper/simple-nft.clar 73.33% <ø> (ø)
core-contracts/contracts/multi-miner.clar 97.05% <ø> (ø)
core-contracts/contracts/subnet.clar 94.96% <ø> (+0.81%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

obycode commented 1 year ago

Also add RegisterAsset op. 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.

obycode commented 1 year ago

I'll look into these failures today.

obycode commented 1 year ago

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?

kantai commented 1 year ago

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?

l1_observer_test is probably the closest to good existing place. You could add a sibling module to tests::l1_observer_test like tests::l1_withdrawal which just imports the necessary utility methods, etc. from l1_observer_test.

obycode commented 1 year ago

The last commit looks unrelated, but it is just cleaning up a bunch of warnings that were annoying during the build/test process.

obycode commented 1 year ago

Ok @kantai, this is ready for a final review.

@zone117x please also take a look at the updated events and let me know if that will be okay for the API or if you request any changes.

obycode commented 1 year ago

Thanks @kantai - I made that CI change in the latest commit.

@zone117x I added the txid to the event as requested. Can you take one more look at that?