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

Withdraw ERC1155 and EVM based chains support added for NFT PoC #1704

Merged
merged 91 commits into from
Mar 24, 2023

Conversation

laruh
Copy link
Member

@laruh laruh commented Mar 9, 2023

related to #900

  • implements withdraw_erc1155 method
  • support for Avalanche, Fantom, Polygon chains was added. Use uppercase for chain names.

If there is no amount param for withdraw_nft, amount = 1 by default.

withdraw_nft req for ERC1155 token without amount param
{
  "userpass": "'$USERPASS'",
  "method": "withdraw_nft",
  "mmrpc": "2.0",
  "params": {
    "type": "WithdrawErc1155",
    "withdraw_data": {
      "chain": "POLYGON",
      "from": "0xf622a6C52C94b500542E2AE6bcAD24C53Bc5b6a2",
      "to": "0x6FAD0eC6bb76914b2a2a800686acc22970645820",
      "token_address": "0x48c75fbf0452fa8ff2928ddf46b0fe7629cca2ff",
      "token_id": "5"
    }
  }
}
withdraw_nft res for ERC1155 token without amount param
{
  "mmrpc": "2.0",
  "result": {
    "tx_hex": "f9014c808526f52f5416830109b49448c75fbf0452fa8ff2928ddf46b0fe7629cca2ff80b8e4f242432a000000000000000000000000f622a6c52c94b500542e2ae6bcad24c53bc5b6a20000000000000000000000006fad0ec6bb76914b2a2a800686acc229706458200000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000023078000000000000000000000000000000000000000000000000000000000000820136a0f8b50c8a98ac1b550aad13532e999ad128343f10d4269a64de242fb4c338c42aa022fbdb9a7570ef8513ab7654e49beb71a20372aec4c61e3d85ec7a81403f6f3a",
    "tx_hash": "fe8d5fc480c534625d7e6fc81eadff8b8f6c986cd46f00d5993b6e21d6bd28c9",
    "from": [
      "0xf622a6C52C94b500542E2AE6bcAD24C53Bc5b6a2"
    ],
    "to": [
      "0x6FAD0eC6bb76914b2a2a800686acc22970645820"
    ],
    "contract_type": "ERC1155",
    "token_address": "0x48c75fbf0452fa8ff2928ddf46b0fe7629cca2ff",
    "token_id": "5",
    "amount": "1",
    "fee_details": {
      "type": "Eth",
      "coin": "MATIC",
      "gas": 68020,
      "gas_price": "0.000000167322276886",
      "total_fee": "0.01138126127378572"
    },
    "coin": "MATIC",
    "block_height": 0,
    "timestamp": 1678269896,
    "internal_id": 0,
    "transaction_type": "NftTransfer"
  },
  "id": null
}
send_raw_transaction req
{
  "method": "send_raw_transaction",
  "coin": "MATIC",
  "tx_hex": "f9014c808526f52f5416830109b49448c75fbf0452fa8ff2928ddf46b0fe7629cca2ff80b8e4f242432a000000000000000000000000f622a6c52c94b500542e2ae6bcad24c53bc5b6a20000000000000000000000006fad0ec6bb76914b2a2a800686acc229706458200000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000023078000000000000000000000000000000000000000000000000000000000000820136a0f8b50c8a98ac1b550aad13532e999ad128343f10d4269a64de242fb4c338c42aa022fbdb9a7570ef8513ab7654e49beb71a20372aec4c61e3d85ec7a81403f6f3a",
  "userpass": "'$USERPASS'"
}
send_raw_transaction res
{"tx_hash":"fe8d5fc480c534625d7e6fc81eadff8b8f6c986cd46f00d5993b6e21d6bd28c9"}

polygonscan result

Another withdraw_nft request example with max param.

withdraw_nft req for ERC1155 with max:true
{
  "userpass": "'$USERPASS'",
  "method": "withdraw_nft",
  "mmrpc": "2.0",
  "params": {
    "type": "WithdrawErc1155",
    "withdraw_data": {
      "chain": "POLYGON",
      "from": "0xf622a6C52C94b500542E2AE6bcAD24C53Bc5b6a2",
      "to": "0x6FAD0eC6bb76914b2a2a800686acc22970645820",
      "token_address": "0x48c75fbf0452fa8ff2928ddf46b0fe7629cca2ff",
      "token_id": "5",
      "max": true
    }
  }
}
withdraw_nft res for ERC1155 with max:true
{
  "mmrpc": "2.0",
  "result": {
    "tx_hex": "f9014c01852c61bc8e85830109b49448c75fbf0452fa8ff2928ddf46b0fe7629cca2ff80b8e4f242432a000000000000000000000000f622a6c52c94b500542e2ae6bcad24c53bc5b6a20000000000000000000000006fad0ec6bb76914b2a2a800686acc229706458200000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000023078000000000000000000000000000000000000000000000000000000000000820136a0af066015590d840579baf398c4bd5e46b2dd71daaa3eae5fbb9909e30834edeea02ba9754f643571920eec1cab076f5f74e1cb5e2879d6fdaea7f90d8e43505655",
    "tx_hash": "9c32db57fdcfd0ccdae193c895cf709aaac5aef1582da825579ddbb831c1f700",
    "from": [
      "0xf622a6C52C94b500542E2AE6bcAD24C53Bc5b6a2"
    ],
    "to": [
      "0x6FAD0eC6bb76914b2a2a800686acc22970645820"
    ],
    "contract_type": "ERC1155",
    "token_address": "0x48c75fbf0452fa8ff2928ddf46b0fe7629cca2ff",
    "token_id": "5",
    "amount": "2",
    "fee_details": {
      "type": "Eth",
      "coin": "MATIC",
      "gas": 68020,
      "gas_price": "0.000000190618308229",
      "total_fee": "0.01296585732573658"
    },
    "coin": "MATIC",
    "block_height": 0,
    "timestamp": 1678351719,
    "internal_id": 0,
    "transaction_type": "NftTransfer"
  },
  "id": null
}
send_raw_transaction req
{
  "method": "send_raw_transaction",
  "coin": "MATIC",
  "tx_hex": "f9014c01852c61bc8e85830109b49448c75fbf0452fa8ff2928ddf46b0fe7629cca2ff80b8e4f242432a000000000000000000000000f622a6c52c94b500542e2ae6bcad24c53bc5b6a20000000000000000000000006fad0ec6bb76914b2a2a800686acc229706458200000000000000000000000000000000000000000000000000000000000000005000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000023078000000000000000000000000000000000000000000000000000000000000820136a0af066015590d840579baf398c4bd5e46b2dd71daaa3eae5fbb9909e30834edeea02ba9754f643571920eec1cab076f5f74e1cb5e2879d6fdaea7f90d8e43505655",
  "userpass": "'$USERPASS'"
}
send_raw_transaction res
{"tx_hash":"9c32db57fdcfd0ccdae193c895cf709aaac5aef1582da825579ddbb831c1f700"}

polygonscan result

@laruh
Copy link
Member Author

laruh commented Mar 17, 2023

Q:Why do we have count field in NftList ?

Oh, it is from previous nft list / history struct versions, when there were different nft lists grouped by chain.
Since each Nft struct has Chain type field, it is not necessary to count nfts in chain groups, bcz we dont have such groups anymore.
I removed count from NftList and NftsTransferHistoryList.

@laruh laruh added under review and removed in progress Changes will be made from the author labels Mar 17, 2023
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thanks you for the fixes! Next iteration

mm2src/coins/eth.rs Outdated Show resolved Hide resolved
}

#[cfg(feature = "enable-nft-integration")]
fn get_valid_eth_withdraw_addresses(
Copy link
Collaborator

@shamardy shamardy Mar 21, 2023

Choose a reason for hiding this comment

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

get_valid_eth_withdraw_addresses is used only for NFT so the name should probably be more indicative of this. I actually think there should be a common implementation for both withdraw NFT functions that gets used in both withdraw_erc721, withdraw_erc1155 since there is a lot of common code between them, this will make this function not needed and the code below will be included in only the common withdraw NFT impl.
Please consider this refactor in the next PRs.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed to get_valid_nft_add_to_withdraw.
Yeah, there is a need for refactoring, noted it

…aw-erc1155

# Conflicts:
#	mm2src/coins/eth.rs
@laruh laruh added in progress Changes will be made from the author and removed under review labels Mar 22, 2023
@laruh laruh added under review and removed in progress Changes will be made from the author labels Mar 22, 2023
shamardy
shamardy previously approved these changes Mar 22, 2023
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Great Work 🔥

Only a non-blocker below and this #1704 (comment)

mm2src/coins/eth.rs Outdated Show resolved Hide resolved
@laruh
Copy link
Member Author

laruh commented Mar 23, 2023

@ozkanonur Hi, its ready for next review iteration.

onur-ozkan
onur-ozkan previously approved these changes Mar 23, 2023
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM, only a few 'not so important' suggestions

mm2src/coins/nft.rs Outdated Show resolved Hide resolved
mm2src/coins/eth.rs Outdated Show resolved Hide resolved
mm2src/coins/eth.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Re-approve

@shamardy shamardy merged commit 4d691ca into dev Mar 24, 2023
@shamardy shamardy deleted the NFT-integration-withdraw-erc1155 branch March 24, 2023 18:04
@ca333 ca333 mentioned this pull request Mar 27, 2023
@laruh laruh mentioned this pull request Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants