Closed telezhnaya closed 2 years ago
FYI: the discussion could be both here or after https://github.com/near/NEPs/issues/254#issuecomment-934487778
awesome. why do we want to add permissions to log authors? because this is new. even if we wanted to, it's impossible to check permissions based on logs that anyone can send.
@evergreen-trading-systems it will be a complete anarchy. Without simple check, anyone can write any contract with any logs and make fake transfers, and we will believe these logs. Even if noone is interested in it because of financial reasons, be sure that we will have a disaster on each April 1st. This is too easy to hack. I agree that we can trust the authors of the contract, but we should distinguish somehow between the author and unknown contract.
Just wanted to clarify: I know that the source of truth is the contract itself, and nothing will change because of fake logs, but people seem to trust Wallet and Explorer, and I don't want to provide them with the random data.
UPD: emitted_by_contract_account_id
could be always taken from receipt_receiver_account_id, and that would be the natural way of checking the author of logs. Updated the message above
UPD:
emitted_by_contract_account_id
could be always taken from receipt_receiver_account_id, and that would be the natural way of checking the author of logs. Updated the message above
Yes I think this makes sense. To be able to filter by contract account id that's emitting the event. I think somewhere down the line, consuming apps, when the Near nodes start doing logs/events with bloom filters, people will do something like the following rpc call.
near.get_Logs({ blockNumberStart:0, blockNumberEnd:100, contract_accounts:["cooldapp.near", "awesomeDapp.near"], type: json, topic: "nft_mint"})
and under the hood the nodes use bloom filters to make all the mechanics work efficiently.
The emitted_by_contract_account_id I could see as a prelude to later interfaces that might pull this data. I'm also pro the additional optional fields.
@frol you may receive spam notifications from here. I removed you from the reviewers, I will return you when the code will be ready
We also need to think about loading historical data. I suggest collecting the current state of all known NFT contracts by https://nomicon.io/Standards/NonFungibleToken/Enumeration.html#interface As a result, we will have a state without previous history of movements.
@frol sorry for such a massive number of changed files: cargo clippy
becomes stricter, as I understand.
Important things since your last check:
src/retriable.rs
so that it's possible to pass custom error handling functionawait_retry_or_panic
, it invoked the logic 1 times less than expected. If you invoked it with 1 attempt, it gave you an immediate error.cargo clippy
warnings
I mentioned most of the things in the doc inside the SQL script, please have a look at that. Additionally, I want to help the contract creators to find the relevant information. We discussed the format of event logs here
From the event log, we (as Indexer developers) should be able to retrieve the following information:
token_id
event_kind
(mint, transfer, burn), reminder: we decided to group by thistoken_sender_account_id
(should be skipped for mint)token_receiver_account_id
(should be skipped for burn)token_transfer_approval_account_id
(optional, see the info below, TODO we haven't discussed that)token_transfer_memo
(optional, feel free not to fill it inside your batch event log if it's empty or you are worried about log size)token_transfer_approval_account_id
https://nomicon.io/Standards/NonFungibleToken/ApprovalManagement.html You have to fill in this information in the event log if it's relevant for the current transfer. Let's looks at the example: If Alice approves Bob to transfer her token, and then Bob transfers it to Maggie, then the fields should look like this:
The same situation, but Alice as the owner does the transfer by herself:
We DO NOT store
called_method
anywhere because we get the info from logs, the method could be retrieved from receipt if you need. @MaximusHaximus do you need this info in Wallet?@frol @MaximusHaximus @mikedotexe @evergreen-trading-systems @zcstarr @robert-zaremba