Enable out-of-order transaction processing for `asset` table #77

danenbm · 2023-05-24T21:31:26Z

Notes

Update owner and delegate in asset table when collection or creator verification occurs.
Modify program transformers to enable out-of-order transaction processing by adding the following changes:
- Upsert leaf info based on seq OR was_decompressed flag.
- Upsert owner and delegate based on owner_delegate_seq.
- Upsert "compression status" fields (compressed, compressible, supply, and supply_mint) based on was_decompressed flag.

Database changes

Add owner_delegate_seq and was_decompressed columns to asset table, and PROGRAMMABLE_NFT to specification_asset_class type.
- Regenerate Sea ORM objects.
- Needed to temporarily comment out a performance improvement migration that removed foreign keys so that SeaORM CLI would regenerate, since it builds relations based on those constraints.
- Needed to add a migration to add PROGRAMMABLE_NFT to specification_asset_class since it is needed by the API code but missing from the SQL code.

Other misc. changes

Fix docker preparation script to build solana-program-library.
Also remove unused 'filling' variable.
Rustfmt/clippy fixes as I change files.

Testing

Made sure it builds and runs in Docker.
Read API specific test: Ran a sequence of CreateTree/Mint, Transfer, and Burn in reverse order. While running in reverse order I observed that could not get info for the asset via getAsset until Mint was indexed.
Used transaction forwarder to send out sequences of transactions in correct and reverse order, and in each case observe cl_items and asset table in database. Covered every indexed instruction (note MintToCollection indexes as Mint). Did this process on both main branch and this PR branch and compared all results.

Transaction sequence	Forwards on main	Backwards on main	Forwards on PR branch	Backwards on PR branch
`CreateTree`/`Mint`, `Transfer`, and `Burn`	OK	`cl_items` OK, `asset` incorrect due to only indexing `Mint`	OK	OK
`CreateTree`/`Mint`, `Redeem`, `CancelRedeem`, second `Redeem`, and `Decompress`	`cl_items` and`asset` incorrect due to `Redeem` not indexing	`cl_items` incorrect due to `Redeem` not indexing, `asset` incorrect due to only indexing `Mint`	OK	OK
`CreateTree`/`Mint`, `Transfer`, second `Transfer`	OK	`cl_items` OK, `asset` incorrect due to only indexing `Mint`	OK	OK
`CreateTree`/`Mint`, `Delegate`, `Transfer`	OK	`cl_items` OK, `asset` incorrect due to only indexing `Mint`	OK	OK
`CreateTree`/`Mint`, `VerifyCreator`	OK	`cl_items` OK, `asset` incorrect due to only indexing `Mint`, `asset_creators` incorrect due to missing `VerifyCreator`	OK	OK, fixed in #87 ~~`cl_items` and `asset` OK, `asset_creators` incorrect due to missing `VerifyCreator`~~
`CreateTree`/`Mint`, `VerifyCollection`	`cl_items` and `asset` OK, `asset_grouping` incorrect due to missing `VerifyCollection`	`cl_items` OK, `asset` incorrect due to only indexing `Mint`, `asset_grouping` incorrect due to missing `VerifyCollection`	OK, fixed in #90. `SetAndVerifyCollection` still needs Blockbuster fix to index. ~~cl_items and asset OK, asset_grouping incorrect due to missing VerifyCollection~~	OK, fixed in #90. `SetAndVerifyCollection` still needs Blockbuster fix to index. ~~cl_items and asset OK, asset_grouping incorrect due to missing VerifyCollection~~

…erification occurs

* This allows out-of-order Bubblegum transactions to create and update the asset table. * Upsert leaf schema, owner, delegate, and seq separately since those are updated by all instructions and gated by sequence number to ensure freshest value. * Mint, burn, and decompress happen without regard to sequence number because they operate on unique fields. * Mint and burn have been updated but Decompress still needs to be fixed to handle out of order transactions. * Also remove unused 'filling' variable.

NicolasPennie · 2023-05-26T04:24:09Z

What's the motivation for this PR? Was anyone observing indexing issues due to out of order transactions?

danenbm · 2023-05-26T17:16:56Z

What's the motivation for this PR? Was anyone observing indexing issues due to out of order transactions?

@NicolasPennie what I heard from @austbot is that both you (Helius) and Triton had observed issues with out of order transactions and as a result, had to essentially stop running the backfiller and use a different method for in-order filling. I also talked to @austbot and @linuskendall about this and it does in fact look like this reference code could not handle out of order transactions in general.

The cl_table looks like it can handler out of order transactions, but the asset table does not. The reason the cl_table can handle out of order transactions is because all instructions except for burn and decompress update the same leaf schema fields, and use the tree sequence number to prevent stale data.

For example, if a transfer was ingested followed by a mint (out of order), the change log will be inserted into cl_items for the transfer, and then when the mint is ingested, it won't update the fields in the table with stale data (owner, delegate, etc.) because the sequence number for the mint will be a lower number than the sequence number for the transfer that was already ingested.

However, using this same example but considering the asset table, what happens is when the transfer comes in first, is the asset is not found in the table, because the mint did not get indexed yet. So the update fails and logs a message and the indexer continues. Later when the mint instruction is indexed, it inserts the row for the asset into the asset table. Now the asset table contains the asset with a stale owner, delegate, and leaf hash.

This PR addresses this issue with the asset table, allowing for out-of-order Bubblegum transactions to be processed in general. I am still looking at the edge cases and how this works for non-compressed NFTs.

NicolasPennie · 2023-05-26T17:27:47Z

What's the motivation for this PR? Was anyone observing indexing issues due to out of order transactions?

@NicolasPennie what I heard from @austbot is that both you (Helius) and Triton had observed issues with out of order transactions and as a result, had to essentially stop running the backfiller and use a different method for in-order filling. I also talked to @austbot and @linuskendall about this and it does in fact look like this reference code could not handle out of order transactions in general.

The cl_table looks like it can handler out of order transactions, but the asset table does not. The reason the cl_table can handle out of order transactions is because all instructions except for burn and decompress update the same leaf schema fields, and use the tree sequence number to prevent stale data.

For example, if a transfer was ingested followed by a mint (out of order), the change log will be inserted into cl_items for the transfer, and then when the mint is ingested, it won't update the fields in the table with stale data (owner, delegate, etc.) because the sequence number for the mint will be a lower number than the sequence number for the transfer that was already ingested.

However, using this same example but considering the asset table, what happens is when the transfer comes in first, is the asset is not found in the table, because the mint did not get indexed yet. So the update fails and logs a message and the indexer continues. Later when the mint instruction is indexed, it inserts the row for the asset into the asset table. Now the asset table contains the asset with a stale owner, delegate, and leaf hash.

This PR addresses this issue with the asset table, allowing for out-of-order Bubblegum transactions to be processed in general. I am still looking at the edge cases and how this works for non-compressed NFTs.

Great, thanks for the context! That was really helpful.

austbot

reviewed on a call with md

austbot · 2023-06-05T14:00:45Z

digital_asset_types/src/dao/generated/sea_orm_active_enums.rs

@@ -116,10 +106,20 @@ pub enum SpecificationAssetClass {
    Print,
    #[sea_orm(string_value = "PRINTABLE_NFT")]
    PrintableNft,
+    #[sea_orm(string_value = "PROGRAMMABLE_NFT")]
+    ProgrammableNft,
    #[sea_orm(string_value = "TRANSFER_RESTRICTED_NFT")]


can remove later

Why is that, because we won't ever use it?

austbot · 2023-06-07T19:57:18Z

this is LGTM @danenbm

danenbm · 2023-06-07T22:34:16Z

this is LGTM @danenbm

Thank you. Still testing this and working through some issues with upserting parts of an asset row due to NULL constraints on some of the fields. Putting back to draft until I fix them

nft_ingester/src/program_transformers/bubblegum/mint_v1.rs

nft_ingester/src/program_transformers/bubblegum/db.rs

… now)" This reverts commit 2713a18.

README.md

nft_ingester/src/program_transformers/bubblegum/burn.rs

nft_ingester/src/program_transformers/bubblegum/cancel_redeem.rs

nft_ingester/src/program_transformers/bubblegum/db.rs

NicolasPennie · 2023-06-23T21:50:36Z

nft_ingester/src/program_transformers/bubblegum/transfer.rs

+                )
+                .await?;
+
+                upsert_asset_with_seq(txn, id_bytes.to_vec(), seq as i64).await


Is this necessary? I think we can just update this within one of the earlier methods. We try to reduce to number of DB calls we need to make in favour of performance. (applies for all cNFT indexing methods).

In a previous iteration of this PR I used the existing seq to protect leaf updates, and the new owner_delegate_seq to protect owner and delegate fields. Because of this, an effect of my PR changes is that when the leaf was set to None during decompression, so was the seq, and then for decompressed assets, a Read API call to getAsset would return null for the seq.

Maybe that was OK because people don't need the seq when the asset has been removed from the merkle tree. But the merkle tree is still listed and wanted it to match previous behavior.

Maybe I can change it back if we want to remove an extra update that is only for the seq.

a Read API call to getAsset would return null for the seq.

This would be acceptable if the asset can never be re-compresssed. But I think you plan on adding that, right? If that's the case, then it makes sense to keep the seq value available after a decompression.

Yeah I think long term we plan to add re-compression. Maybe it would insert back into the same tree like cancel_redeem does but instead of using a voucher it would burn the token-metadata based NFT. But I could also see it just doing a new append to the tree via mint_v1.

Either way when it got added back into the tree, I think the asset would get a new seq that is probably a much higher number that what was previously stored there when it was decompressed. I guess what I'm thinking at the moment is that I'm not sure of the use for the seq after decompression since you cannot use it for re-compression, right?

For reference the previous version of my PR prior to this commit used seq to protect leaf and owner_delegate_seq to protect owner+delegate and had one less upsert overall. May be worth revisiting that approach to save on an update/upsert.

I guess what I'm thinking at the moment is that I'm not sure of the use for the seq after decompression since you cannot use it for re-compression, right?

If recompressing will add that NFT back into the tree via the same leaf, then we need to keep the seq number around. Otherwise you can have out of order bugs with out of order decompress + recompress transactions. And we need to be able to always handle any order to be safe, even if its unlikely to happen.

I think you uncovered something that will be an issue for decompress > recompress with the current design both of this PR and with the system in general. The fact that decompress doesn't have any association with the tree, means there is no change log event and thus no seq associated with the decompress.

For this PR, the way I found to deal with decompress being out of order with others was to add a boolean was_decompressed flag, which essentially acts like a 1-bit sequence number, which is a one way door assuming no events after the decompression.

If we add recompress we need to find another way to deal with decompress being out of order with other instructions that can provide a seq from the tree.

@NicolasPennie @danenbm we must keep the seq in the db if the asset is recompressed.
there are other things though, since you cant close the mint acccount you cannot fully recompress. so you can use that as a handle some how.

Leaving code as is to make sure seq stays updated in asset table.

However, I still don't think recompression can work out of order, because decompression is not sequenced by the tree. Am I missing something?

nft_ingester/src/program_transformers/bubblegum/mint_v1.rs

nft_ingester/src/program_transformers/bubblegum/redeem.rs

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

nft_ingester/src/program_transformers/bubblegum/db.rs

…" (metaplex-foundation#78) This reverts commit bb0a5a0.

danenbm added 2 commits May 24, 2023 14:27

Fix docker preparation script to build SPL

ff389cc

Update owner and delegate in asset table when collection or creator v…

9299d79

…erification occurs

danenbm force-pushed the danenbm/update-asset branch from 39c630c to 9299d79 Compare May 24, 2023 21:34

danenbm requested review from austbot, NicolasPennie and linuskendall May 25, 2023 22:57

danenbm added 4 commits May 26, 2023 13:28

Update mint and decompress to be able to upsert asset info out of order

63b1e48

Add second sequence number for compression status fields

bee5977

Reduce logging in docker

69f9cc3

Comment out compressed_seq before regenerating Sea ORM objects

1dfdfd0

danenbm force-pushed the danenbm/update-asset branch from 6285923 to 1dfdfd0 Compare May 26, 2023 21:48

Merge branch 'main' into danenbm/update-asset

ba6b323

danenbm force-pushed the danenbm/update-asset branch from 330f7b8 to 103a2ae Compare June 1, 2023 22:50

danenbm added 2 commits June 2, 2023 08:57

Add migration for asset specification

507b393

Update README

634030d

danenbm force-pushed the danenbm/update-asset branch from 103a2ae to 634030d Compare June 2, 2023 15:58

Rename PNFT and regenerate Sea ORM types

07b6712

danenbm force-pushed the danenbm/update-asset branch from 98b5be0 to 07b6712 Compare June 2, 2023 17:19

danenbm changed the title ~~Danenbm/update asset~~ Enable out-of-order transaction processing for asset table Jun 2, 2023

Apply usage of compressed_seq after regenerating Sea ORM types

238b043

danenbm marked this pull request as ready for review June 2, 2023 17:44

austbot approved these changes Jun 5, 2023

View reviewed changes

danenbm marked this pull request as draft June 7, 2023 22:34

Merge branch 'main' into danenbm/update-asset

aa6fe0d

Remove column updates in asset table during mint for items not in model

4c0e259

linuskendall reviewed Jun 21, 2023

View reviewed changes

nft_ingester/src/program_transformers/bubblegum/mint_v1.rs Show resolved Hide resolved

danenbm added 4 commits June 21, 2023 22:59

Clippy fixes in ingester main

2b4e5e6

Merge branch 'main' into danenbm/update-asset

ff0e987

Cleanup debug comment

843dd6b

Merge branch 'main' into danenbm/update-asset

f13f7d5

linuskendall mentioned this pull request Jun 22, 2023

Enable out-of-order transaction processing for asset table rpcpool/digital-asset-rpc-infrastructure#83

Closed

Allow for sequence number to be NULL (needed after decompress now)

2713a18

danenbm commented Jun 23, 2023

View reviewed changes

nft_ingester/src/program_transformers/bubblegum/db.rs Outdated Show resolved Hide resolved

danenbm added 2 commits June 23, 2023 12:22

Add leaf specific sequence number to protect that field in asset table

f7a5143

Revert "Allow for sequence number to be NULL (needed after decompress…

bf2d2bc

… now)" This reverts commit 2713a18.

NicolasPennie reviewed Jun 23, 2023

View reviewed changes

danenbm added 2 commits June 27, 2023 14:18

Merge branch 'main' into danenbm/update-asset

eeb56ac

Merge branch 'main' into danenbm/update-asset

7c9e1c1

NicolasPennie reviewed Jul 7, 2023

View reviewed changes

nft_ingester/src/program_transformers/bubblegum/redeem.rs Outdated Show resolved Hide resolved

Update nft_ingester/src/program_transformers/bubblegum/redeem.rs

5b7a33d

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

danenbm mentioned this pull request Jul 13, 2023

Change creator and collection verification to use upserts to support out of order #87

Merged

Merge branch 'main' into danenbm/update-asset

365a9bc

ghost mentioned this pull request Jul 18, 2023

Enable out of order processing helius-labs/digital-asset-rpc-infrastructure#96

Closed

ghost reviewed Jul 19, 2023

View reviewed changes

nft_ingester/src/program_transformers/bubblegum/db.rs Show resolved Hide resolved

Merge branch 'main' into danenbm/update-asset

1de90db

danenbm mentioned this pull request Jul 24, 2023

Fix collection verification and change to use upserts to support out of order processing #90

Merged

danenbm merged commit 8f82f42 into main Jul 26, 2023

muhitrhn pushed a commit to muhitrhn/digital-asset-rpc-infrastructure that referenced this pull request Oct 6, 2023

add logging and change the metric name (metaplex-foundation#77)

bb0a5a0

muhitrhn pushed a commit to muhitrhn/digital-asset-rpc-infrastructure that referenced this pull request Oct 6, 2023

Revert "add logging and change the metric name (metaplex-foundation#77)…

73642eb

…" (metaplex-foundation#78) This reverts commit bb0a5a0.

danenbm deleted the danenbm/update-asset branch October 9, 2023 06:07

danenbm restored the danenbm/update-asset branch October 9, 2023 06:07

danenbm deleted the danenbm/update-asset branch October 9, 2023 06:07

This was referenced Nov 7, 2023

Feat: Out of order updates: if we see a missing mint, push that to be backfilled #109

Closed

Potential issue with redeem indexing of transactions #103

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable out-of-order transaction processing for `asset` table #77

Enable out-of-order transaction processing for `asset` table #77

danenbm commented May 24, 2023 •

edited

Loading

NicolasPennie commented May 26, 2023

danenbm commented May 26, 2023 •

edited

Loading

NicolasPennie commented May 26, 2023

austbot left a comment

austbot Jun 5, 2023

danenbm Jun 22, 2023

austbot commented Jun 7, 2023

danenbm commented Jun 7, 2023 •

edited

Loading

NicolasPennie Jun 23, 2023

danenbm Jun 26, 2023

NicolasPennie Jun 27, 2023 •

edited

Loading

danenbm Jun 27, 2023 •

edited

Loading

danenbm Jun 27, 2023

NicolasPennie Jun 28, 2023

danenbm Jun 28, 2023 •

edited

Loading

austbot Jul 10, 2023

danenbm Jul 13, 2023 •

edited

Loading

Enable out-of-order transaction processing for asset table #77

Enable out-of-order transaction processing for asset table #77

Conversation

danenbm commented May 24, 2023 • edited Loading

Notes

Database changes

Other misc. changes

Testing

NicolasPennie commented May 26, 2023

danenbm commented May 26, 2023 • edited Loading

NicolasPennie commented May 26, 2023

austbot left a comment

Choose a reason for hiding this comment

austbot Jun 5, 2023

Choose a reason for hiding this comment

danenbm Jun 22, 2023

Choose a reason for hiding this comment

austbot commented Jun 7, 2023

danenbm commented Jun 7, 2023 • edited Loading

NicolasPennie Jun 23, 2023

Choose a reason for hiding this comment

danenbm Jun 26, 2023

Choose a reason for hiding this comment

NicolasPennie Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

danenbm Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

danenbm Jun 27, 2023

Choose a reason for hiding this comment

NicolasPennie Jun 28, 2023

Choose a reason for hiding this comment

danenbm Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

austbot Jul 10, 2023

Choose a reason for hiding this comment

danenbm Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Enable out-of-order transaction processing for `asset` table #77

Enable out-of-order transaction processing for `asset` table #77

danenbm commented May 24, 2023 •

edited

Loading

danenbm commented May 26, 2023 •

edited

Loading

danenbm commented Jun 7, 2023 •

edited

Loading

NicolasPennie Jun 27, 2023 •

edited

Loading

danenbm Jun 27, 2023 •

edited

Loading

danenbm Jun 28, 2023 •

edited

Loading

danenbm Jul 13, 2023 •

edited

Loading