feat: core indexing #168
feat: core indexing #168
Conversation
There was a problem hiding this comment.
Great job. Aside from the main logic, just minor comments. And looks like we're 1 step away from dropping the sea_orm dependency altogether, just the enums are left to be removed.
Please don't forget to add reingest labels when we're modifying the data structures.
Also a question: should we reingest the existing database, or write a migration to modify those 2 columns? What'll be an effort to write such migration?
blockbuster/src/programs/mpl_core_program/mod.rsWORKING_20240311
Outdated
Show resolved
Hide resolved
nft_ingester/tests/api_tests.rs
Outdated
| amount: 1, | ||
| amount: 0, |
There was a problem hiding this comment.
Check: This seems unrelated to everything else in the PR. Was it supposed to change?
| @@ -314,4 +319,161 @@ mod tests { | |||
| panic!("expected MasterEdition enum variant"); | |||
| }; | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
| async fn mpl_core_update_process() { | |||
There was a problem hiding this comment.
Long term probably want to add a test with the Royalties plugin with Creators, as well as an item in a Collection. However I recommend waiting to add more tests until the CompressionProof type is replaced though with a newer type.
# Conflicts: # nft_ingester/src/mpl_core_processor.rs # rocks-db/src/asset.rs
# Conflicts: # rocks-db/src/asset.rs
| plugins: dynamic_data | ||
| .plugins | ||
| .clone() | ||
| .map(|plugins| serde_json::from_str(&plugins.value).unwrap_or(serde_json::Value::Null)), | ||
| unknown_plugins: dynamic_data | ||
| .unknown_plugins | ||
| .clone() | ||
| .map(|plugins| serde_json::from_str(&plugins.value).unwrap_or(serde_json::Value::Null)), | ||
| num_minted: dynamic_data | ||
| .num_minted | ||
| .clone() | ||
| .map(|num_minted| num_minted.value), | ||
| current_supply: dynamic_data | ||
| .current_size | ||
| .clone() | ||
| .map(|current_size| current_size.value), | ||
| plugins_json_version: dynamic_data | ||
| .plugins_json_version | ||
| .clone() | ||
| .map(|plugins_json_version| plugins_json_version.value), |
There was a problem hiding this comment.
Not sure if you want to prefex the mpl-core stuff with mpl_core_. I did that in the reference implementation demo and was planning on doing that in classic DAS so that it was clear that these fields did not have anything to do with Bubblegum or Token Metadata assets.
There was a problem hiding this comment.
Its looking good to me with the latest updates. Note I have ported the changes to the plublic repos now and plan to work on those implementations going forward.
Here is the classic DAS PR: metaplex-foundation/digital-asset-rpc-infrastructure#178
Once this Blockbuster PR gets merged I will release Blockbuster and then we will be able to use the released crate and remove it from this repo:
metaplex-foundation/blockbuster#41
# Conflicts: # postgre-client/src/lib.rs
# Conflicts: # entities/Cargo.toml # nft_ingester/src/config.rs # nft_ingester/src/message_handler.rs
| tx_info: &plerkle::TransactionInfo, | ||
| ) -> VecDeque<(IxPair, Option<Vec<IxPair>>)> { | ||
| order_instructions( | ||
| &KEY_SET, |
There was a problem hiding this comment.
Why do we put here KEY_SET with Bubblegum, TokenMetadata, SPLToken and Core program keys if then we iterate throug all of this instructions and process only Bubblegum related?
There was a problem hiding this comment.
Thank you, dropped unnecessary keys!
| @@ -445,9 +448,7 @@ impl BubblegumTxProcessor { | |||
| let mut asset_update = AssetUpdateEvent { | |||
| ..Default::default() | |||
| }; | |||
| let tree_id = Pubkey::new_from_array(tree_id.to_owned()); | |||
| // Pubkey::new_from_array(bundle.keys.get(3).unwrap().0.to_vec().try_into().unwrap()); | |||
There was a problem hiding this comment.
NIT: can we drop these comments?
feat: rent_epoch field + rocks migrators + old DAS params compatibility
# Conflicts: # nft_ingester/src/bin/ingester/main.rs
migrations/2_interface_extend.sql
Outdated
| @@ -0,0 +1,4 @@ | |||
| COMMIT; | |||
There was a problem hiding this comment.
This looks like a very dirty hack.
There was a problem hiding this comment.
I agree)) I'll try to find something better
There was a problem hiding this comment.
Just as a suggestion: https://stackoverflow.com/a/56376907
There was a problem hiding this comment.
but that'll be quite heavy on our db, may probably take over an hour given the indexes update (I'm not 100% sure of this)
No description provided.