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

Bubblegum Update Metadata Version 1 #134

Merged
merged 49 commits into from
Jan 9, 2024
Merged

Conversation

danenbm
Copy link
Contributor

@danenbm danenbm commented Oct 17, 2023

Overview

Detailed notes

  • Similar to our previous approach for protecting against out-of-order/concurrent updates, it adds sequence numbers to the asset table and asset_data table for data that can be updated by both mint_v1 and update_metadata.
  • To prevent overwriting data for decompressed assets, there were previously slot_updated checks on several pieces of data. These worked because the data was only written for compressed assets by mint_v1, and then after decompression only updated by Token Metadata indexing through account-based updates. In order to support update_metadata's ability to change some of the same asset data, these slot_updated checks were replaced with sequence number checks that allow for out-of-order indexing for compressed assets before and after they are decompressed.
  • To handle multiple background tasks downloading off-chain metadata, a conditional check was added to the background task that makes sure the URI matches what is in the database before doing the http download (to avoid wasted work), and then again as a condition when updating the database with the downloaded data (to prevent updating the asset with data from a stale URI with concurrent updates). This fixes both a potential race condition with existing account-based updates for Token Metadata, and is also now required for compressed assets because update_metadata can change the URI and be indexed concurrently with mint_v1.
  • Changes to the creator array for both compressed and non-compressed assets are now handled by upserting creators (using an ON CONFLICT with assetId and position columns). Bubblegum supports an empty creator array, and in that case a zero-length creator is inserted into the asset_creators table to allow for a sequence number (and thus handle out-of-order updates).
    • The reason the new approach does not delete existing creators and then re-insert updated creators, as was previously done with non-compressed assets, is that we found it could lead to race conditions where stale creators were inserted by concurrent updates.
    • To avoid the race condition we could have added an explicit lock or used the serializable transaction isolation level in Postgres. But to prevent indexing performance impacts we chose the no-lock solution with filtering on the API side.

Sea-ORM autogeneration

As mentioned offline, there were some slight discrepancies with the existing generated Sea-ORM code and the migrations, so I updated the generated code to match the migrations.

Alternate version

I created a Version 2 of this PR that overall uses less new sequence numbers, but required more refactoring and making sure mint_v1 and update_metadata both update all of the metadata fields.

The problem with Version 2 is that with multiple ingester processes, there can be race conditions. So sticking with this version, although incorporating some of the changes from Version 2

Regression testing

Tested with latest commit: 36df8e5

  • Tested following instruction sequences from Bubblegum and verified database was in correct state afterwards:
    • create_tree, mint_v1, transfer, burn
    • create_tree, mint_v1, redeem, decompress_v1
    • create_tree, mint_v1, redeem, cancel_redeem, redeem, decompress_v1
    • create_tree, mint_v1, transfer, transfer
    • create_tree, mint_v1, delegate, transfer
    • create_tree, mint_v1, verify_creator
    • create_tree, mint_v1, verify_collection
    • create_tree, mint_v1, verify_collection, unverify_collection
    • create_tree, mint_v1, set_and_verify_collection
    • create_tree, mint_to_collection_v1, unverify_collection
  • Deleted and rebuilt database, ran the instruction sequences from Bubblegum in reverse order (i.e.burn, transfer, mint_v1, create_tree, ) and verified database ended up in same state as when they were run forwards (decompressed items have slightly different expected state when run in reverse order).
  • Also ran several of the sequences in non-sequential order and duplicated some of the transactions, and still got expected results.

Update Metadata testing

Tested with latest commit: 36df8e5

  • Ran a createTree, mintV1, verifyCreator, unverifyCreator, and updateMetadata, and observed after the updateMetadata that the name, symbol, and URI had been changed.
  • Deleted the database, then ran the exact same 5 transactions in reverse and observed exact same final result in the database.
  • Again, ran a createTree, mintV1, verifyCreator, unverifyCreator, and updateMetadata, checking database state between each transaction.
    • Verified the second creator was set to verified after verifyCreator. Verified getAsset showed same result.
    • Verified the correct creator was set to unverified after unverifyCreator. Verified getAsset showed same result.
    • Verified that before updateMetadata, that getAssetsByCreator returned the asset when either of the two creators was used.
    • Verified that the name, symbol, URI, and creator array changed after updateMetadata, to the values specified in the updateArgs sent into the instruction.
    • Verified that after updateMetadata was sent, only the creator that was in the updateArgs was shown by a call to getAsset (the "stale" creator from asset_creators was NOT reported).
    • Also verified that after updateMetadata was sent, that getAssetsByCreator only returned the asset when the remaining creator was used. Using the "stale" creator from the database did not result in a returned asset.

@danenbm danenbm force-pushed the danenbm/update-metadata-parsing branch from c826583 to 0240ce5 Compare October 19, 2023 04:49
@danenbm danenbm force-pushed the danenbm/update-metadata-parsing branch from 58ef341 to 213dac5 Compare October 20, 2023 22:21
@danenbm danenbm changed the title Bubblegum Update Metadata Bubblegum Update Metadata Version 1 Oct 23, 2023
@danenbm danenbm marked this pull request as ready for review October 23, 2023 11:08
@danenbm danenbm requested review from NicolasPennie, linuskendall, Juanito87, febo and blockiosaurus and removed request for linuskendall October 23, 2023 11:08
* Use new Blockbuster that always updates all
creators and verification status.
* Remove deleting creators with lower sequence
numbers as it would not work due to race
conditions.
* Add concept of "empty" creator value to
support Bubblegum empty creator arrays.
* Add filtering out of old creators or having
no creators to DAS code.
* Also get authority and tree_id accounts from
Bubblegum during mint and update_metadata.
@danenbm danenbm force-pushed the danenbm/update-metadata-parsing branch from c9dda08 to 8803152 Compare December 18, 2023 10:33
@danenbm danenbm force-pushed the danenbm/update-metadata-parsing branch from a3e9ddd to 3dc3557 Compare December 21, 2023 09:48
@danenbm danenbm changed the base branch from update-metadata-staging to main December 28, 2023 22:20
@danenbm danenbm requested a review from linuskendall January 3, 2024 06:49
@danenbm danenbm merged commit 2d5a0dc into main Jan 9, 2024
2 checks passed
@danenbm danenbm deleted the danenbm/update-metadata-parsing branch January 18, 2024 22:42
@danenbm danenbm restored the danenbm/update-metadata-parsing branch January 18, 2024 22:42
@danenbm danenbm deleted the danenbm/update-metadata-parsing branch January 18, 2024 22:43
kespinola referenced this pull request in rpcpool/digital-asset-rpc-infrastructure Jan 26, 2024
* Update raw name and raw symbol for existing NFTs (#139)

Update the raw name and raw symbol from the onchain data for existing NFTs when reingesting. This allows correction of incorrect values during reprocessing on an existing index.

* Upstream Helius features (#133)

* Bubblegum Update Metadata Version 1 (#134)

* Add code to index Bubblegum Update Metadata

* Update rust toolchain file

* Fix moved variable after merge

* Add code from mintV1 that allows for empty URI

* Ordering using asset.seq initially applied to update_metadata

* Add simple check for whether asset was decompressed to Bubblegum transformers

* Don't prevent sequence number update when already decompressed

* Add sequence number to downloading metadata background task

* Add sequence number migration (Sea ORM not regenerated yet)

* Regenerate Sea-ORM types

* Use new sequence numbers for Bubblegum Update Metadata

* Extra condition to protect out of order creator verification

* Remove base_info_seq for each creator and add creators_added_seq to asset table

* Regenerate Sea-ORM types

* Change creator metadata updates to use new creators_added_seq

* Factor out common creator update code to helper function

* Update to latest blockbuster beta

* Use less than or equal for download metadata seq check

* Index verified for token metadata collection

* Add slot_updated to initial asset upsert, and removed duplicate items

* Remove asset_was_decompressed

Replaced with WHERE clauses on each upsert.
Move remaining upserts from mint_v1 to db.rs.
Remove upsert to asset_v1_account_attachments from mint_V1.
Combine upserts for asset base info and royalty amount.

* Rename royalty_amount_seq to base_info_seq

* Fix typo in WHERE clause

* Do not delete existing creators in mint_v1

* Update comments around database txns

* Use transaction in mint_V1 and update_metadata

* Use transaction for other Bubblegum instructions asset table updates

* Fix tree_id key index in update_metadata

* Remove use of was_decompressed flag on asset table

* Add migration to remove was_decompressed and regenerate SeaORM types

* Combine upsert_asset_base_info and upsert_creators and add lock

* Remove unneeded creators_added_seq

* Switch to EXCLUSIVE mode lock

* Add NULL condition check on asset.seq

* Refactored creator indexing

* Use new Blockbuster that always updates all
creators and verification status.
* Remove deleting creators with lower sequence
numbers as it would not work due to race
conditions.
* Add concept of "empty" creator value to
support Bubblegum empty creator arrays.
* Add filtering out of old creators or having
no creators to DAS code.
* Also get authority and tree_id accounts from
Bubblegum during mint and update_metadata.

* Add conditions to creator upsert, add another check at DAS API level

* Rename asset_creators.verified_seq back to just regular seq

* Remove unneeded condition on asset_authority upsert

* Apply stale creator filtering to all DAS API queries

* Use latest blockbuster beta release

* Remove download_metadata_seq and add URI match check instead

* Fix task URI initial query

* Regenerate Sea ORM types without download_metadata_seq

* asset_grouping.verified option remove

* Fix filtering for getAssetsByCreator

* Update to blockbuster 0.9.0-beta.5 and mpl-bubblegum 1.0.1-beta.4

* Configurable account streams (#148)

* Make workers configurable

Make workers fully configurab le and remove reference to the plerkle plugin.

* fix lifetime

---------

Co-authored-by: Kirill Fomichev <[email protected]>

* fix: splt tokens with no token stanard are incorrectly categorized as single. Logic in token account updates would change owner when any token account had amount > 0 would triggers spam updates of the owner of the asset with any transfer. (#151)

* Improve workspace usage (#141)

* fix: remove decompress ix handling and use db transactions (#156)

* Improvements for consistency NFT indexing and query consistency (#157)

* Danenbm/bubblegum sequence tests 2 (#160)

* Add script to forward transactions an check database results

* Fix ordering and add debug info

* Add remaining non-creator/non-collection tests

* Require asset and cl_items files to exist

* Add asset_creators and asset_grouping tests

* Add verify_creator and verify_collection tests

* Add more collection verification tests

* Move test data to subirectory

* Move repeated code to functions

* Add support for running sequences in reverse

* Add instructions to README for running test script

* Minor README update

* feat: `getSignaturesForAsset` endpoint on top of new `cl_audits_v2` table (#155)

* add migration files for cl_audits_v2

* add types

* ingester

* add getSignaturesForAsset endpoint

* refactor to resolve merge conflict related bugs

* address clippy error

* rename to get_asset_signatures

* add instruction type update_metadata

* add error log if instruction is unknown

* add sort order changes

---------

Co-authored-by: Nicolas Pennie <[email protected]>

* fix ORM circular dependency (#161)

* fix ORM circular dependency

* PR comments

---------

Co-authored-by: Linus Kendall <[email protected]>
Co-authored-by: Tahsin Tunan <[email protected]>
Co-authored-by: Michael Danenberg <[email protected]>
Co-authored-by: Kirill Fomichev <[email protected]>
Co-authored-by: Nicolas Pennie <[email protected]>
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.

4 participants