near / near-indexer-for-explorer

Watch NEAR network and store all the data from NEAR blockchain to PostgreSQL database
https://near-indexers.io/docs/projects/near-indexer-for-explorer
GNU General Public License v3.0
123 stars 56 forks source link

NFT event assumes predecessor_id for NFT contract #191

Closed willemneal closed 2 years ago

willemneal commented 2 years ago

This line shows where this assumption is used.

https://github.com/near/near-indexer-for-explorer/blob/d6c88a71811b75666b0ad6999b7c1da5c7b2d487/src/db_adapters/assets/non_fungible_token_events.rs#L107

However, this isn't valid because it could be the case that the contract allows another account to mint the NFT. For example, it costs X NEAR to anyone to mint.

Is it worth having an extra column for receiver_id? I'm also unclear the full context of this data. Is this for a particular receipt? For example, the initial call is to another contract that calls the NFT to mint. So would the receiver here be the initial contract or the NFT contract? Is it for the individual promise receipt or the full transaction?

Personally I would find it useful to have both columns since in my use case it distinguishes a normal mint vs. one from a linkdrop, but this isn't too important to have if saving space in the table is important.

telezhnaya commented 2 years ago

it could be the case that the contract allows another account to mint the NFT

We can't check that fact. If we will trust anyone here, people will mint the tokens in their small contracts, saying they are minting them in large and prestigious ones. If we are talking about your case, the contract should definitely know that someone minted the token on their behalf, it means that the contract can provide all necessary logs in a callback function.

Is it worth having an extra column for receiver_id?

I'm not sure I fully understand this question. Am I right, that you suggest new column and then argue about it? Or you are talking about token_new_owner_account_id?

I'm OK with the idea of adding any information, one new column surely will not damage the DB size, but the column should bring the value and be consistent with all other data stored in the DB.

willemneal commented 2 years ago

There is no trust needed. The log is always made by the NFT contract. I would argue that most of the time predecessor_id is not the contract. For example, the typical case is that the owner of the NFT contract calls mint. In this case the predecessor_id is the owner_id and not the NFT's account_id. You could design the mint to be a private callback on the contract to ensure that the predecessor is the contract. However, we cannot expect everyone to do this nor is this a good idea.

However, in all of these cases the receiver_id of the call is the contract. This is why I asked about the context of the receipt data.

Consider account A, contracts, B & NFT,, and --> means calls. If A --> B --> NFT.mint. In terms of the initial transaction the receiver is B, but in the context of promise the receiver is NFT. So if the receipt context is the latter then receiver_id should be used instead of predecessor_id in all cases.

In my use case if you claim a linkdrop you are using a function call access key to mint so the predecessor_id is the contract. So having this information makes it easy to count the linkdrops claimed. But it's not crucial information. So my point is that we could have columns for both receiver and predecessor, but the latter is not needed most of the time.

The same logic could be applied to the other events. If I wanted to transfer a token, then my account_id would be the predecessor_id.

frol commented 2 years ago

@telezhnaya Willem is right, we should use receiver_id as the contract_id there.

telezhnaya commented 2 years ago

@willemneal now I see. Thank you for catching it and for the PR!