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

Feature: Process ERC-721 information #19

Merged
merged 5 commits into from
Dec 18, 2023

Conversation

brunomenezes
Copy link
Collaborator

Summary

Code changes to support indexing information from ERC-721 deposits coming from ERC721Portal contract

I created an ERC-721 contract and deployed it on Sepolia to transfer and see the real thing going through.
Contract Address: 0x7a3Cc9c0408887a030A0354330C36a9cd681AA7E
OpenSea: testnet/collection

PS: Now honeypot has a NFT on Sepolia, cartesiScan/apps

Changelog:

  • Added the ABI for ERC-721.
  • Added new database migration.
  • Added new graphQL schema types and a few relationships between them.
  • Added logic to handle input-added when sender is the ERC721Portal, also in case name and symbol is not available (I think it is hard to happen, but it is covered nonetheless)
  • Added test cases for the aforementioned points.

@brunomenezes brunomenezes linked an issue Dec 12, 2023 that may be closed by this pull request
4 tasks
schema.graphql Outdated Show resolved Hide resolved
const erc20Deposit = await this.handlePayload(input, block, ctx);
if (erc20Deposit) {
this.depositStorage.set(inputId, erc20Deposit);
ctx.log.info(`${inputId} (Erc20Deposit) stored`);
input.erc20Deposit = erc20Deposit;
}

input.erc721Deposit = await this.prepareErc721Deposit(
Copy link
Member

Choose a reason for hiding this comment

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

this block of code stayed a little bit different from the token handling above. Is there a way to make it more uniform?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the handlePayload() deserves a bit of refactoring following the steps of prepareErc721Deposit in a separate PR. When I started, I did add the logic inside it. I would return an object with a return like {erc721Deposit: ERC20 } | {erc20Deposit: NFT} so it would be mutually exclusive because the msgSender is 1:1. Also, I was not fond of the duplication on checking if it exists than store/log/assign. I prefer to separate and have a method to take care of everything at once and leave the handle() to control and glue for the Input. That is for the context where I am coming from. I can create an issue and refactor that in a separate branch.

@brunomenezes brunomenezes force-pushed the feature/18-process-erc-721-information branch from 94d6846 to d6f236b Compare December 12, 2023 05:29
@brunomenezes brunomenezes requested a review from tuler December 12, 2023 23:38
@brunomenezes brunomenezes merged commit 80002e1 into main Dec 18, 2023
2 checks passed
@brunomenezes brunomenezes deleted the feature/18-process-erc-721-information branch December 18, 2023 02:25
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.

Process ERC-721 information
3 participants