[FA migration] Dynamically process FA mapping #698

bowenyang007 · 2025-01-27T21:39:26Z

Summary

We were getting the FA mapping via a hardcoded list of top 100 coins. Now we want to generate this dynamically. The idea is to:

Maintain a mapping of v1 coin type to fungible asset address (derived from coin type)
Persist that mapping in a new table called coin_to_fungible_asset_mapping
When starting processor, bootstrap the mapping from postgres table.
Update and use mapping locally
Store new entries into postgres for next time.

This strategy works because the mapping is guaranteed to be idempotent.

Also to be absolutely correct we're still using the constants file. Only if it can't find it in the constants file will we look in the dynamic mapping.

[WARNING] This only applies to processors built on top of SDK because we can't easily persist a mapping with the old code.

Backfill

Genesis because we need to create the mapping table
Testnet: 0
Mainnet: 0

Testing

codecov · 2025-01-27T21:51:09Z

Codecov Report

Attention: Patch coverage is 77.10438% with 68 lines in your changes missing coverage. Please review.

Project coverage is 49.7%. Comparing base (32af360) to head (617c462).

Files with missing lines	Patch %	Lines
...ocessor/src/processors/fungible_asset_processor.rs	71.4%	32 Missing ⚠️
...t_fungible_asset_processor/parquet_fa_extractor.rs	0.0%	17 Missing ⚠️
...t_models/raw_v2_fungible_asset_to_coin_mappings.rs	83.7%	7 Missing ⚠️
...uet_processors/parquet_fungible_asset_processor.rs	0.0%	4 Missing ⚠️
.../fungible_asset_processor/fungible_asset_storer.rs	85.1%	4 Missing ⚠️
...ngible_asset_processor/fungible_asset_extractor.rs	86.9%	3 Missing ⚠️
rust/integration-tests/src/models/fa_v2_models.rs	0.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main    #698     +/-   ##
=======================================
+ Coverage   49.5%   49.7%   +0.1%     
=======================================
  Files        252     254      +2     
  Lines      28535   28765    +230     
=======================================
+ Hits       14149   14315    +166     
- Misses     14386   14450     +64

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

just-in-chang

few comments but overall looks good

rust/processor/src/db/postgres/migrations/2025-01-23-164631_coin_infos_for_migration/up.sql

just-in-chang · 2025-01-30T23:59:57Z

rust/sdk-processor/src/processors/fungible_asset_processor.rs

+        let mut fa_extractor = FungibleAssetExtractor::new();
+        fa_extractor
+            .bootstrap_coin_to_fa_mapping(self.db_pool.clone())
+            .await?;


Maybe instead just call the bootstrapping method in the new method/merge the new and bootstrap methods? And just pass the db pool into the new method

i thought about it but it's the norm to have new not async. In this case it's probably a nonissue but I can imagine some cases (e.g. singleton) where that delineation avoids bugs.

just-in-chang · 2025-01-31T00:03:00Z

rust/integration-tests/src/models/fa_v2_models.rs

+#[derive(Clone, Debug, Deserialize, FieldCount, Identifiable, Insertable, Serialize)]
+#[diesel(primary_key(coin_type))]
+#[diesel(table_name = coin_to_fungible_asset_mappings)]
+pub struct CoinToFungibleAssetMapping {


why do we need to duplicate this struct for tests lol

I guess they include whatever here for testing. If it's not here then it won't be used in tests.

the current paradigm is for those models with inserted at column, seems like this table doesn't, then we can probably reuse the model struct defined in the db.

just-in-chang · 2025-01-31T00:07:51Z

...ocessor/src/db/common/models/fungible_asset_models/raw_v2_coin_to_fungible_asset_mappings.rs

+pub trait CoinToFungibleAssetMappingConvertible {
+    fn from_raw(raw: RawCoinToFungibleAssetMapping) -> Self;
+}


Don't think we need an explicit trait just for this

Maybe. I'm just following the existing paradigm. I agree that it feels a bit redundant vs just using From. Let's keep discussing in slack.

good point @just-in-chang yeah since we don't do any batch conversion,and will have distinct structs for parquet and postgres models I think we can simply use From here instead. I can do as a part of refactoring I am doing

just-in-chang · 2025-01-31T00:08:49Z

...processor/src/db/postgres/models/fungible_asset_models/v2_coin_to_fungible_asset_mappings.rs

+impl CoinToFungibleAssetMappingConvertible for CoinToFungibleAssetMapping {
+    fn from_raw(raw: RawCoinToFungibleAssetMapping) -> Self {
+        Self {
+            coin_type: raw.coin_type,
+            fungible_asset_metadata_address: raw.fungible_asset_metadata_address,
+            last_transaction_version: raw.last_transaction_version,
+        }
+    }
+}


Instead of having a custom trait, u could just impl From<RawCoinToFungibleAssetMapping> for CoinToFungibleAssetMapping and call .into() when needed

nvm seems like all the other models have a trait like this lol... dunno why but ok

...ocessor/src/db/common/models/fungible_asset_models/raw_v2_coin_to_fungible_asset_mappings.rs

rust/sdk-processor/src/steps/parquet_fungible_asset_processor/parquet_fa_extractor.rs

rust/sdk-processor/src/steps/fungible_asset_processor/fungible_asset_extractor.rs

just-in-chang · 2025-01-31T00:23:52Z

rust/processor/src/processors/fungible_asset_processor.rs

+    let mut kv_mapping: CoinToFungibleAssetMappings = AHashMap::new();
+    for mapping in data {
+        kv_mapping.extend(mapping);
+    }


Can try .into_par_iter().fold() https://docs.rs/rayon/latest/rayon/iter/struct.Fold.html

hmm this is also the existing paradigm let's fix it together in a different PR lol.

yuunlimm · 2025-01-31T05:37:20Z

rust/integration-tests/src/models/events_models.rs

-        // TODO: make this more generic to load all fields, then we should be able to run tests for all processor in one test case.
-
+* this is created b/c there is inserted_at field which isn't defined in the Event struct, we can't just load the events directly without specifying the fields.
+* TODO: make this more generic to load all fields, then we should be able to run tests for all processor in one test case.


nit: TODO comment can be removed

yuunlimm · 2025-01-31T05:40:05Z

rust/integration-tests/src/models/fa_v2_models.rs

+#[derive(Clone, Debug, Deserialize, FieldCount, Identifiable, Insertable, Serialize)]
+#[diesel(primary_key(coin_type))]
+#[diesel(table_name = coin_to_fungible_asset_mappings)]
+pub struct CoinToFungibleAssetMapping {


the current paradigm is for those models with inserted at column, seems like this table doesn't, then we can probably reuse the model struct defined in the db.

yuunlimm · 2025-01-31T05:42:18Z

...ocessor/src/db/common/models/fungible_asset_models/raw_v2_coin_to_fungible_asset_mappings.rs

+        }
+    }
+
+    /// Get the asset_type_v1 (coin type) from either the mapping or the constant mapping


nit: comment was a bit confusing to me. either the dynamic mapping table or the static ?

yuunlimm · 2025-01-31T05:48:26Z

...ocessor/src/db/common/models/fungible_asset_models/raw_v2_coin_to_fungible_asset_mappings.rs

+pub trait CoinToFungibleAssetMappingConvertible {
+    fn from_raw(raw: RawCoinToFungibleAssetMapping) -> Self;
+}


good point @just-in-chang yeah since we don't do any batch conversion,and will have distinct structs for parquet and postgres models I think we can simply use From here instead. I can do as a part of refactoring I am doing

bowenyang007 requested review from rtso and yuunlimm January 27, 2025 21:39

bowenyang007 changed the title [] [FA migration] Dynamically process FA mapping Jan 27, 2025

rtso approved these changes Jan 27, 2025

View reviewed changes

bowenyang007 requested a review from rtso January 30, 2025 07:30

rtso approved these changes Jan 30, 2025

View reviewed changes

bowenyang007 requested a review from just-in-chang January 30, 2025 22:02

bowenyang007 marked this pull request as ready for review January 30, 2025 23:33

just-in-chang reviewed Jan 31, 2025

View reviewed changes

yuunlimm reviewed Jan 31, 2025

View reviewed changes

bowenyang007 added 2 commits January 30, 2025 23:34

create schema for new mapping table

f318c03

add handling for the new mapping table

b7bdd16

bowenyang007 force-pushed the fix_balance branch from 3256603 to b7bdd16 Compare January 31, 2025 20:01

bowenyang007 added 4 commits January 31, 2025 15:33

added integration test

2a1e851

lint

ba545ec

add additional flags for skipping tables

e990667

fix bug and clean up

617c462

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FA migration] Dynamically process FA mapping #698

[FA migration] Dynamically process FA mapping #698

bowenyang007 commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading

just-in-chang left a comment

just-in-chang Jan 30, 2025

bowenyang007 Jan 31, 2025

just-in-chang Jan 31, 2025

bowenyang007 Jan 31, 2025

yuunlimm Jan 31, 2025

just-in-chang Jan 31, 2025

bowenyang007 Jan 31, 2025

yuunlimm Jan 31, 2025

just-in-chang Jan 31, 2025

just-in-chang Jan 31, 2025

bowenyang007 Jan 31, 2025

just-in-chang Jan 31, 2025

bowenyang007 Jan 31, 2025

yuunlimm Jan 31, 2025

yuunlimm Jan 31, 2025

yuunlimm Jan 31, 2025

yuunlimm Jan 31, 2025

[FA migration] Dynamically process FA mapping #698

Are you sure you want to change the base?

[FA migration] Dynamically process FA mapping #698

Conversation

bowenyang007 commented Jan 27, 2025 • edited Loading

Summary

Backfill

Testing

codecov bot commented Jan 27, 2025 • edited Loading

Codecov Report

just-in-chang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bowenyang007 commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading