Skip to content
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

NFT RPC Methods Docs update #270

Merged
merged 14 commits into from
Aug 23, 2024
Merged

Conversation

harsenyan3
Copy link
Collaborator

No description provided.

@gcharang gcharang requested review from laruh and gcharang July 3, 2024 12:10
Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! first iteration.

For the future, please create new feature branches from dev with appropriate names that reflect the feature you are working on, instead of simply checking out dev.

I noticed that the documentation is missing the enable_nft RPC. Its purpose is similar to the enable_erc20 request for token activation when the platform coin has already been activated, but it specifically applies to nft type tokens. This RPC activates the global NFT instance, which contains a list of NFTs owned by the user.

req example looks like this (if proxy true)

curl --url "http://127.0.0.1:7783" --data '{
  "userpass": "'$USERPASS'",
  "method": "enable_nft",
  "mmrpc": "2.0",
  "params": {
    "ticker": "NFT_MATIC",
    "activation_params": {
      "provider":{
        "type": "Moralis",
        "info": {
          "url":"http://localhost:6150/nft-test",
          "proxy_auth":true
        }
      }
    }
  }
}'

provider field "Defines available NFT providers and their configuration."
proxy_auth os Optional

"POLYGON"
],
"url": "https://moralis-proxy.komodo.earth",
"url_antispam": "https://nft.antispam.dragonhound.info"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose you removed this brace accidentally, making the JSON invalid.
you can re check it in this site https://codebeautify.org/jsonviewer

right now "https://moralis-proxy.komodo.earth" doesnt have auth validation and rate limits, when we deploy up to date proxy layer with these changes KomodoPlatform/komodo-defi-proxy#22 on moralis-proxy.komodo.earth domain or other type, "proxy_auth": true should be added
@smk

as for now, after adding proxy Optional info, there is no need to add changes in this RPC

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readded the brace.

Comment on lines 17 to 18
| maker\_swap\_v2\_contract | string | Backup address for the maker's swap smart contract |
| taker\_swap\_v2\_contract | string | Backup address for the taker's swap smart contract |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swap_v2 addresses are not backup, right now we have legacy swap protocol and implementing the new one, calling it trading protocol upgrade KomodoPlatform/komodo-defi-framework#1895
swap_contract_address field is a legacy adress of etomic swap smart contract.

v2 swap contracts are related to etomic swap contracts implemented for trading protocol upgrade v2
https://github.com/KomodoPlatform/etomic-swap/tree/5e15641cbf41766cd5b37b4d71842c270773f788/contracts

maker_swap_v2_contract, taker_swap_v2_contract and nft_maker_swap_v2_contract addresses are
fields of deployed EtomicSwapMakerV2, EtomicSwapTakerV2 and EtomicSwapMakerNftV2 contracts
https://github.com/KomodoPlatform/etomic-swap/tree/5e15641cbf41766cd5b37b4d71842c270773f788/contracts

Optional "swap_v2_contracts" field was added in enable_eth_with_tokens RPC
KomodoPlatform/komodo-defi-framework#2129 (comment)
in rust code it looks like this

pub swap_v2_contracts: Option<SwapV2Contracts>

pub struct SwapV2Contracts {
    pub maker_swap_v2_contract: Address,
    pub taker_swap_v2_contract: Address,
    pub nft_maker_swap_v2_contract: Address,
}

if "use_trading_proto_v2"is true in mm config (json config which you provide when start program) then swap_v2_contracts must be provided

so update the request fields.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also you should provide info about optional "nft_req" field in activation request

in rust code this field looks like this, it can help you with object names and description in documentation

nft_req: Option<NftActivationRequest>,

/// Encapsulates the request parameters for NFT activation, specifying the provider to be used.
#[derive(Clone, Deserialize)]
pub struct NftActivationRequest {
    pub provider: NftProviderEnum,
}

/// Defines available NFT providers and their configuration.
#[derive(Clone, Deserialize)]
#[serde(tag = "type", content = "info")]
pub enum NftProviderEnum {
    Moralis {
        url: Url,
        #[serde(default)]
        proxy_auth: bool,
    },
}

so in json request the rust code looks like

    "nft_req": {
      "provider":{
        "type": "Moralis",
        "info": {
          "url":"http://localhost:6150/nft-test",
          "proxy_auth":true
        }
      }
    }

Indicated proxy_auth is optional and defaults to false.
Readded BSC chain
fixed nft_req
Created enable_nft file (based on enable_erc20)
Comment on lines +40 to +56
```json
{
"mmrpc": "2.0",
"result": {
"balances": {
"0x0d317904AF3BA3A993d557b6cba147FEA4DeB57E": {
"spendable": "0",
"unspendable": "0"
}
},
"platform_coin": "ETH",
"token_contract_address": "0x0d8775f648430679a709e98d2b0cb6250d2887ef",
"required_confirmations": 3
},
"id": null
}
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀 this is wrong response for enable nft

Added NftProvider object to common structures
Changed activation_param definition to be a standard NftProvider object
@smk762 smk762 changed the base branch from dev to update/nft-methods August 23, 2024 04:51
@smk762 smk762 merged commit 394bc21 into KomodoPlatform:update/nft-methods Aug 23, 2024
4 checks passed
@smk762 smk762 mentioned this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants