Add proxy support by laruh · Pull Request #1775 · GLEECBTC/komodo-defi-framework

laruh · 2023-05-02T08:29:20Z

url for proxy service was added in requests
possible_spam field was added in Nft and NftTransferHistory structures
make nft feature default

request examples
get_nft_list

{
  "userpass": "'$USERPASS'",
  "method": "get_nft_list",
  "mmrpc": "2.0",
  "params": {
    "chains": [
      "BSC",
      "POLYGON"
    ],
    "url": "https://moralis-proxy.komodo.earth"
  }
}

get_nft_transfers

{
  "userpass": "'$USERPASS'",
  "method": "get_nft_transfers",
  "mmrpc": "2.0",
  "params": {
    "chains": [
      "POLYGON"
    ],
    "url": "https://moralis-proxy.komodo.earth"
  }
}

get_nft_metadata

{
  "userpass": "'$USERPASS'",
  "method": "get_nft_metadata",
  "mmrpc": "2.0",
  "params": {
    "token_address": "0x5c7d6712dfaf0cb079d48981781c8705e8417ca0",
    "token_id": "0",
    "chain": "BSC",
    "url": "https://moralis-proxy.komodo.earth"
  }
}

withdraw erc721

{
  "userpass": "'$USERPASS'",
  "method": "withdraw_nft",
  "mmrpc": "2.0",
  "params": {
    "url": "https://moralis-proxy.komodo.earth",
    "withdraw_type": {
      "type": "withdraw_erc721",
      "withdraw_data": {
        "chain": "BSC",
        "to": "0x6FAD0eC6bb76914b2a2a800686acc22970645820",
        "token_address": "0xfd913a305d70a60aac4faac70c739563738e1f81",
        "token_id": "214300044414"
      }
    }
  }
}

withdraw erc1155

{
  "userpass": "'$USERPASS'",
  "method": "withdraw_nft",
  "mmrpc": "2.0",
  "params": {
    "url": "https://moralis-proxy.komodo.earth",
    "withdraw_type": {
      "type": "withdraw_erc1155",
      "withdraw_data": {
        "chain": "POLYGON",
        "to": "0x6FAD0eC6bb76914b2a2a800686acc22970645820",
        "token_address": "0x48c75fbf0452fa8ff2928ddf46b0fe7629cca2ff",
        "token_id": "5",
        "max": true
      }
    }
  }
}

Note: after adding db support "url" param will be removed from requests above and will be added into new update_nft and refresh_metadata

# Conflicts: # CHANGELOG.md

onur-ozkan

thank you for taking the previous notes serious, I have a few notes regarding to changes

mm2src/coins/Cargo.toml

onur-ozkan · 2023-05-02T12:26:23Z

mm2src/coins/nft/nft_structs.rs


+#[derive(Clone, Deserialize)]
+pub struct WithdrawNftReq {
+    pub(crate) url: String,


In what scenarios we would need this field?

It is needed to withdraw erc1155 token type in current implementation.
As amount of erc1155 NFT could be > 1, we need to check that owner has enough "NFT" balance. So I created find_wallet_amount function to access info about all nfts owned by the user.

ps: I cant use get_nft_metadata function, bcz it returns info about the latest owner. ERC1155 can have more than 1 owners, it is fine to call this finction to get the info about name, token_uri, metadata etc since they are common to erc1155 tokens with identical token_id, but owner in response could be different, so amount could be different.

I mean why it's not hardcoded in the mm2 side? We don't really need to change the url as far as I know. I don't know how this field can be useful ever. If we migrate to gui-auth, all we need to do update the DNS for current proxy address.

I just thought that hard coding is a bad practice and providing url in request enable people to use their own proxy if they want. But if you ok with hard coded endpoint as it was I can return it.

Yeah, this seems reasonable. But as far as I see this is send for each withdraw request, unlike other urls

btw I don't have hard feeling on this - we can ship this with it. It just didn't look quite right to me(sending url with each request)

It just didn't look quite right to me(sending url with each request)

No worry when db support will be added only update_nft and refresh_metadata API methods will use url, bcz only these methods will be sending requests to moralis to get new info.
GUI told that its fine for them to change requests a little bit I will just need to ping them and provide up to date info about req and res forms.

But as far as I see this is send for each withdraw request, unlike other urls

Well yeah I ask url for all erc types in withdraw method, but use it only for "type":"withdraw_erc1155", I just didnt want to add new param in withdraw_erc1155 structure. Once db will be added I just remove url, other fields will be the same.

Usually RPC urls should be included in the enable calls and saved in the coin's fields as part of a client struct, but NFT is a special case since we don't have an enable call for them and defining a coin configuration to an NFT or NFTs in general makes no sense. I still think defining an enable call for NFTs, and some kind of context/coin or client to save the url/s on enable and maybe other important configs in the future if needed is the optimal solution to this url problem. This enable call can be even part of enable_eth_with_tokens if needed.

@laruh please consider this in future PRs if you think it will make things easier.

@shamardy nice hint, thanks.
personally I dont see using urls in couple future NFT methods as a problem, as NFT is a special case right now.
But yeah I think we cant avoid making some nft configs or adding some enable actions in NFT swaps implementation (related to this thread in previous pr).
I will safe this hint for swaps implementation.

mm2src/coins/nft.rs

onur-ozkan

thank you for the fixes, have some non-blocker small notes

@shamardy please take a look on this PR as well

onur-ozkan · 2023-05-04T09:10:49Z

mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs

 use coins::eth::EthCoin;
 use coins::my_tx_history_v2::my_tx_history_v2_rpc;
-#[cfg(feature = "enable-nft-integration")] use coins::nft;
+use coins::nft;


future note: we will need to create it's own crate for nft

will need to create it's own crate for nft

should think about it, in the future there are swaps with nfts. wouldn't it be strange that nfts are not a part of the coins in this case?

It can inherit from coins, but it's not really a coin. Majority part of the coins modules needs to be seperated to different/their own crates. e.g eth module has amazingly much than what it should have. Which takes development/maintanince/compilation costs to much higher levels.

mm2src/coins/nft.rs

mm2src/coins/lp_coins.rs

shamardy

🚀

shamardy · 2023-05-04T15:51:25Z

mm2src/coins/nft.rs

 }

 #[cfg(target_arch = "wasm32")]
-async fn send_moralis_request(uri: &str, api_key: &str) -> MmResult<Json, GetNftInfoError> {


We should allow API users to use their own API key if they want in the future, this is not a priority now though.

laruh added 4 commits May 2, 2023 15:26

add proxy support

e3540b1

make nft feature default

dcedcb7

CHANGELOG entry

0536be8

Merge remote-tracking branch 'origin/dev' into nft-reverse-proxy

cfece88

# Conflicts: # CHANGELOG.md

laruh requested a review from onur-ozkan May 2, 2023 12:16

laruh added the under review label May 2, 2023

onur-ozkan suggested changes May 2, 2023

View reviewed changes

laruh added in progress and removed under review labels May 3, 2023

laruh added 2 commits May 3, 2023 13:01

remove nft feature flag

7a0e0ef

add MORALIS_API_ENDPOINT

d187b4e

laruh added under review and removed in progress labels May 3, 2023

laruh requested a review from onur-ozkan May 4, 2023 05:31

onur-ozkan previously approved these changes May 4, 2023

View reviewed changes

onur-ozkan requested a review from shamardy May 4, 2023 09:23

laruh added in progress and removed under review labels May 4, 2023

improve readability of the url building

fd0c905

laruh dismissed onur-ozkan’s stale review via fd0c905 May 4, 2023 10:04

laruh added under review and removed in progress labels May 4, 2023

shamardy approved these changes May 4, 2023

View reviewed changes

shamardy merged commit 35595c6 into dev May 4, 2023

shamardy deleted the nft-reverse-proxy branch May 4, 2023 16:19

laruh mentioned this pull request May 5, 2023

ERC721/ERC1155 support adding for EVM protocols #900

Open

shamardy mentioned this pull request May 5, 2023

[release] v1.0.4-beta #1804

Merged

shamardy added 1.0.4-beta and removed under review labels May 5, 2023

Conversation

laruh commented May 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onur-ozkan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laruh May 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

onur-ozkan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

laruh commented May 2, 2023 •

edited

Loading

laruh May 4, 2023 •

edited

Loading

onur-ozkan left a comment •

edited

Loading