refactor: store when NFTs are burned

[crnkovic]

Summary

https://app.clickup.com/t/86dqn7z0r

A while back, we added support for NFT activities. Afterwards, we patched it to also store the burn event (when an NFT is burned). Now, since burned NFTs cannot be "unburned", we can use that knowledge to store the timestamp when the NFT was burned to the nfts table. We can then use this burned_at attribute for situations such as updating NFT metadata, to avoid running these operations on burned NFTs. Also, in the future, if we want to show that NFT is burned in the UI, we can do that without expensive query to find corresponding burn activity.

Potential downsides I see here (for showing "NFT is burned" in the UI) is there are some collections that we don't index activity for. For these, we cannot know when NFTs in a collection were burned.

This is a proof of concept and should be discussed before merging.

Update: applied these changes to RefreshNftMetadata job, a job that finds all NFTs pending metadata refresh, but now so that the job will exclude burned NFTs as they're not relevant anymore.

Syncing burn events for collections that already had their activity indexed

Since we need to fill the gaps of missing burn activity, this PR adds the php artisan collections:fetch-burn-activity CLI command that dispatches a job to only index burn activity. Since we order by activity timestamp, it doesn't matter if they are added with the greater ID or created_at columns. This will only run for collections that previously had their activity indexed (looking at the activity_updated_at column). This will also exclude all collections that we don't sync activity for.

This command is not scheduled, as it will only need to run once for every collection. Activity is upserted, so even if there's some activity previously stored in the database, it will be not be inserted twice.

Checklist

• I checked my UI changes against the design and there are no notable differences
• I checked my UI changes for any responsiveness issues
• I checked my (code) changes for obvious issues, debug statements and commented code
• I opened a corresponding card on Clickup for any remaining TODOs in my code
• I added a short description on how to test this PR (if necessary)
• I added a storybook entry for the component that was added (if necessary)
• Documentation (if necessary)
• Tests (if necessary)
• Ready to be merged

[alfonsobries]

In terms of code, it looks good, and I think it's a relevant detail to have. My only consideration is that we should only add this PR if it's really going to be used for any of the functions you mention like updating NFT metadata, (at least have some tasks in ClickUp to implement it) otherwise I would wait to implement this.

In short: in my opinion, I believe it's an improvement and starting by updating the NFT metadata only for non-burned NFTs seems like a good task to add

[crnkovic]

@alfonsobries applied one use case which is excluding burned NFTs from refreshing NFT metadata. this can potentially run for a lot of NFTs, so this approach might speed up the job runtime. looking into what else i can improve with this burned NFTs concept, but wanna keep the PR light, so will create a card when I discover other use-cases

[oXtxNt9U]

How do we apply this retroactively? I think we need some kind of migration job that loops over any existing activities to sync the burn column.

Might be good to check with real data how many affected NFTs we'd even have (just to have a general idea).

Additionally, a burn generally means that there's no owner anymore. So to be consistent we probably need to set wallet_id to null here or else we might end up with burned NFTs that still have a wallet attached (tbf it should fix itself eventually):

https://github.com/ArdentHQ/dashbrd/pull/493/files#diff-4b4b4cff5fbc12e1e63c802626218821b6e915b034588fe018272513129a4ffbR58

[crnkovic]

How do we apply this retroactively? I think we need some kind of migration job that loops over any existing activities to sync the burn column.

True, would need a job to go through collections, fetch the activity, and store LABEL_BURN events. Will probably be a lot faster, as we only care about this type. Will test and add it.

[crnkovic]

Testing on the collections dump:

• 248/554 collections from demo have burn events, in total of around 116k events, took around 30mins to update all
• 181/417 collections from prod have burn events, in total of around ~500k events, took around 1hr to update all

[oXtxNt9U]

@crnkovic Thanks for the numbers

--

So to summarize:

• We add a new nfts.burned_at column
• Populated based on LABEL_BURN event
• Currently only used to skip metadata refreshes for burned NFTs
• 181/417 collections from prod have burn events, in total of around ~500k events
• To populate the column retroactively we need to run php artisan collections:fetch-burn-activity once
• No UI changes

What about this:

Additionally, a burn generally means that there's no owner anymore. So to be consistent we probably need to set wallet_id to null here or else we might end up with burned NFTs that still have a wallet attached (tbf it should fix itself eventually)

Though not sure if it makes a difference in practice, maybe it's fine to just not bother since it should correct itself anyway after a FetchWalletNfts job.

[crnkovic]

Though not sure if it makes a difference in practice, maybe it's fine to just not bother since it should correct itself anyway after a FetchWalletNfts job.

I agree, this will clear itself. I feel this will require much more testing as we remove wallet_id and I'm kinda scared to push that in this PR, since this PR is already pretty beefy.