KomodoPlatform / komodo-defi-framework

This is the official Komodo DeFi Framework repository
https://komodoplatform.com/en/docs/komodo-defi-framework/
104 stars 94 forks source link

ARRR integration. #927

Open artemii235 opened 3 years ago

artemii235 commented 3 years ago
smk762 commented 2 years ago

A few notes from comparing legacy and v2 best_orders and orderbook:

V2 does not appear to have:

min/max volumes can be derived in v2 orderbook as it is same as base_max_volume / base_min_volume.

In v2 best_orders same as base_min_vol when buy or rel_min_vol when sell - but response does not indicate if action was buy or sell. It might be nice to add that, but not critical as the user making request should already know.

V2 best_orders response returns "is_mine": false for a ZOMBIE order placed from the same mm2 instance. Returns true as expected for v2 orderbook tho.

artemii235 commented 2 years ago

@laruh There are a couple of tasks that I would like to give to you for this integration 🙂

  1. Please read Zcash light client protocol spec https://zips.z.cash/zip-0307 and referenced documentation.
  2. Implement native mode support. It should fill the blocks and wallet database in the same way as light mode does. Basically, you need to support Native client along with grpc_client here: https://github.com/KomodoPlatform/atomicDEX-API/blob/ef9c3189c84b09d17eb3ef27218b9a14acc2b22e/mm2src/coins/z_coin/z_rpc.rs#L296
  3. Implement this TODO https://github.com/KomodoPlatform/atomicDEX-API/blob/ef9c3189c84b09d17eb3ef27218b9a14acc2b22e/mm2src/coins/z_coin.rs#L490 We need multiple lightwalletd URLs support for redundancy purpose. They should work in a round-robin way, similar to how it's done in Electrum client. Ask @SirSevenG to set up more servers because there is only one now :)
sergeyboyko0791 commented 2 years ago

In one of my PRs https://github.com/KomodoPlatform/atomicDEX-API/pull/1405, I changed the names of some methods including init_zcoin, init_zcoin_status and init_zcoin_user_aciton. Basically, I just added namespaces to combine some methods into groups, especially init_<METHOD><_ACTION> into task::<METHOD>::<ACTION>. Thus, init_zcoin => task::enable_zcoin::init init_zcoin_status => task::enable_zcoin::status init_zcoin_user_action => task::enable_zcoin::user_action

Changelog: link

Also I added task::enable_zcoin::cancel to gracefully cancel the coin activation task.

artemii235 commented 2 years ago

@laruh Regarding dockerization:

  1. Please use the following repo as komodod regtest Docker image example https://github.com/artemii235/komodo_regtest_docker.
  2. You need to create an image containing the binary from https://github.com/KomodoPlatform/komodo/tree/ARRR-HTLC-debug branch.
  3. The binary should be started similarly as for ZOMBIE chain, but the name of the coin should be different and it should start in regtest mode.
  4. This way, we will have a ZOMBIE/PIRATE like blockchain started from scratch when Docker image is run.
  5. Then, create z_coin_tests module in mm2src/mm2_main/src/docker_tests.
  6. Run the image in our docker_tests. For reference, use mm2src/mm2_main/src/docker_tests/docker_tests_common.rs::utxo_asset_docker_node. Implement Zcoin activation using this dockerized blockchain.
  7. Move and refactor all z_coin related tests to this module, removing ignore attribute.
laruh commented 1 year ago

If there is a connection problem during Zcoin activation, tonic just propagates connection errors and doesn't try to reconnect, if connection is stabilized.

artemii235 commented 1 year ago

@laruh As I can see, the error happened during activation process, and it was not the thing to test 🙂

Could you please check that background sync process continues in case of connection problems after successful coin activation?

Please also post documentation example of Zcash params file path in request for @smk762.

laruh commented 1 year ago

@artemii235

Could you please check that background sync process continues in case of connection problems after successful coin activation?

Oh, after successful coin activation other processes go fine even if we have connection problems.

Please also post documentation example of Zcash params file path in request for @smk762.

I sent an example in dm to smk, and it was added in this pr

artemii235 commented 1 year ago

@laruh Great! I have also added "checksum verification of Zcash params files." to the checklist. Could you do it this week, please?

shamardy commented 1 year ago

This an unordered checklist for the Browser/WASM support for ARRR item based on my initial research and code inspection:

This is a preliminary checklist based on a fast code inspection of the current ARRR implementation in mm2. There might be some other items that I missed or some of the above items might need more research to decide the best way for wasm support.

onur-ozkan commented 1 year ago

This an unordered checklist for the Browser/WASM support for ARRR item based on my initial research and code

I highly suggest to not add anything further on the zcash fork. We should even remove it and use the upstream directly. We need to find/implement the fixes on our side, rather than forking projects which requires additional cost for maintanince.

If it's not so possible doing that, best thing we can do is asking for a generic layer for storage, so everyone can implement their need using generic traits provided by zcash upstream.

laruh commented 1 year ago

Zcash params need to work with wasm too. They will probably need to be saved in browser storage, to do that we need to implement a way to fetch them like how these scripts do it, verify them and then save them in the browser storage.

Note: All zcash params are required to enable coins in native mode. They are too big for browser I think. For Light client sapling-spend and sapling-output params should be enough. request for tests

{
  "method": "task::enable_z_coin::init",
  "mmrpc": "2.0",
  "params": {
    "ticker": "ZOMBIE",
    "activation_params": {
      "mode": {
        "rpc": "Light",
        "rpc_data": {
          "electrum_servers": [
            {
              "url": "zombie.dragonhound.info:10033"
            }
          ],
          "light_wallet_d_servers": [
            "http://zombie.dragonhound.info:443"
          ]
        }
      },
      "zcash_params_path": "/home/user/TEST_PATH/.zcash-params"
    }
  },
  "userpass": "'$USERPASS'"
}

ps: also you can ping me to provide up to date values for check_point_block so you don't have to wait all day to scan the blocks.

image

shamardy commented 1 year ago

Note: All zcash params are required to enable coins in native mode. They are too big for browser I think. For Light client sapling-spend and sapling-output params should be enough.

Thanks @laruh for the hint, added it to the checklist.

shamardy commented 9 months ago

@borngraced it would be good to add notifications / streaming channel implementation to ARRR/zcoin , can you please make them part of your next sprint plans and add them to this checklist https://github.com/KomodoPlatform/komodo-defi-framework/issues/927#issue-872032775 We can start implementing streaming channel for ARRR after this PR https://github.com/KomodoPlatform/komodo-defi-framework/pull/1957 is merged. we should probably start with balance events, then maybe sync status, etc..