[EI-468] Create v1 migration views #385

rtso · 2024-05-22T21:43:03Z

Description

As part of the v1 table deprecation, replace the v1 tables with views using the v2 tables. This will give the ecosystem a migration window to migrate their queries to the v2 tables and reduces ecosystem impact.

Test plan

Ran the SQL locally

linear · 2024-05-22T21:43:45Z

EI-468 Create v1 migration views

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

ying-w · 2024-05-22T23:30:53Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+        TRUE AS maximum_mutable,
+        TRUE AS uri_mutable,
+        TRUE AS description_mutable,
+        TRUE AS properties_mutable,
+        TRUE AS royalty_mutable,


This is not always TRUE

SELECT maximum_mutable, uri_mutable, description_mutable, properties_mutable, royalty_mutable, COUNT(1) FROM current_token_datas GROUP BY 1,2,3,4,5 ORDER BY 1,2,3,4,5

@bowenyang007 validated that most of these are true, so it's fine to hardcode it to avoid having to migrate the v1 logic over to the v2 processor

these are supposed to be used by marketplaces to indicate which fields are non modifiable. however basically everyone just used one minting contract that sets all of these to true so majority of these ended up being true. and nobody seems to care about these.
my thought is that we can remove them and nothing would happen since I haven’t seen any UI that exposes these. If someone ends up wanting them we could create a new table altogether (similar to royalty and other metadata that aren’t critical).

Field is not nullable, previously these could have different bool values

So 2 options here:

make these nullable and set to null. this changes the schema and could cause some compatibility issues

keep as is.

@ying-w do you prefer one or the other?

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

CapCap · 2024-05-29T04:14:56Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+        transaction_version,
+        owner_address,
+        -- this is mainly for hashing the coin type for primary key
+        encode(sha256(asset_type::bytea), 'hex') as coin_type_hash,


do we actually have sha256 support enabled in alloy?

Is the intention to use this as a join? Because if so need to add this as an index on the underlying table

I ran this in dbeaver and it worked so it seems like it's supported. Also I forgot to add a migration view for coin_infos, which is the only table this could join with. Yes we should add indexes on the underlying tables.

CapCap

a few small concerns, but otherwise lgtm

rtso · 2024-05-30T18:06:35Z

Need to add coin_infos

ying-w · 2024-05-31T18:05:00Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+        amount,
+        table_type_v1 AS table_type,
+        tov.inserted_at,
+        tdv.collection_id AS collection_data_id_hash,


values are different, collection_id != collection_data_id_hash

can you give an example?

similar to coins, logic would be encode( sha256( (collection_creator_address || '::' || token_name)::bytea ), 'hex')

from here

aptos-indexer-processors/python/utils/token_utils.py

Line 44 in 5244b84

hash(f"{standardize_address(self.creator)}::{self.name}")

The processing code does the hash already for v1 collections and tokens

aptos-indexer-processors/rust/processor/src/models/token_v2_models/v2_collections.rs

Line 264 in 48d7794

let collection_id = collection_id_struct.to_id();

So basically everything that's v1 in tdv is already hashed.

just realized that, so just need ltrim

ying-w · 2024-05-31T20:22:35Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+    JOIN collections_v2 cv
+    ON tdv.collection_id = cv.collection_id AND tdv.transaction_version = cv.transaction_version


i dont think this works, token_activities_v2 has new entries when tokens have activities whereas collections_v2 has new entries when collection properties change

i guess the best we can do is joining on the current_ tables

rtso · 2024-06-03T17:36:12Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+        royalty_points_numerator,
+        royalty_points_denominator,


Need to join on new royalty tables

bowenyang007 · 2024-06-04T23:09:13Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+CREATE OR REPLACE VIEW legacy_migration_v1.coin_activities AS
+    SElECT
+        transaction_version,
+        owner_address as event_account_address,
+        -- these two below are mildly concerning
+        0 as event_creation_number,
+        0 as event_sequence_number,
+        owner_address,
+        asset_type AS coin_type,
+        amount,
+        "type" AS activity_type,
+        is_gas_fee,
+        is_transaction_success,
+        entry_function_id_str,
+        block_height,
+        transaction_timestamp,
+        inserted_at,
+        event_index,
+        gas_fee_payer_address,
+        storage_refund_amount,
+    FROM public.fungible_asset_activities
+    WHERE token_standard = 'v1'
+  ;
+CREATE INDEX lm1_ca_ct_a_index ON public.fungible_asset_activities USING btree (asset_type, amount)
+CREATE INDEX lm1_ca_ct_at_a_index ON public.fungible_asset_activities USING btree (asset_type, "type", amount)
+CREATE INDEX lm1_ca_oa_ct_at_index ON public.fungible_asset_activities USING btree (owner_address, asset_type, "type", amount)
+CREATE INDEX lm1_ca_oa_igf_index ON public.fungible_asset_activities USING btree (owner_address, is_gas_fee)


have we tested any of this in terms of performance?

yuunlimm · 2024-06-05T23:19:20Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

+CREATE INDEX lm1_ca_ct_a_index ON public.fungible_asset_activities USING btree (asset_type, amount)
+CREATE INDEX lm1_ca_ct_at_a_index ON public.fungible_asset_activities USING btree (asset_type, "type", amount)
+CREATE INDEX lm1_ca_oa_ct_at_index ON public.fungible_asset_activities USING btree (owner_address, asset_type, "type", amount)
+CREATE INDEX lm1_ca_oa_igf_index ON public.fungible_asset_activities USING btree (owner_address, is_gas_fee)


@bowenyang007 this is where it's failing

yuunlimm · 2024-06-11T05:20:34Z

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql

@@ -52,11 +50,26 @@ SELECT transaction_version,
    inserted_at
 FROM public.fungible_asset_balances
 WHERE token_standard = 'v1';
+-- replace `coin_infos` with `fungible_asset_metadata`
+CREATE OR REPLACE VIEW legacy_migration_v1.coin_infos AS
+SELECT encode(sha256(asset_type::bytea), 'hex'),


do we need to rename this? as coin_type_hash

Yes! Good catch.

rtso changed the title ~~Create v1 migration views~~ [EI-468] Create v1 migration views May 22, 2024

rtso requested review from CapCap, ying-w, bowenyang007, yuunlimm and a team May 22, 2024 21:43

rtso force-pushed the rtso/v1-migration-views branch from 57447fb to 4eeb4f5 Compare May 22, 2024 21:44

ying-w reviewed May 22, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

ying-w reviewed May 22, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

ying-w reviewed May 22, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Show resolved Hide resolved

ying-w reviewed May 22, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

ying-w reviewed May 23, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

CapCap reviewed May 29, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

CapCap reviewed May 29, 2024

View reviewed changes

rust/processor/migrations/2024-05-22-200847_add_v1_migration_views/up.sql Outdated Show resolved Hide resolved

CapCap reviewed May 29, 2024

View reviewed changes

CapCap approved these changes May 29, 2024

View reviewed changes

ying-w reviewed May 31, 2024

View reviewed changes

rtso commented Jun 3, 2024

View reviewed changes

yuunlimm mentioned this pull request Jun 4, 2024

skip writing deprecated tables in default processor #394

Merged

bowenyang007 reviewed Jun 4, 2024

View reviewed changes

yuunlimm reviewed Jun 5, 2024

View reviewed changes

bowenyang007 force-pushed the rtso/v1-migration-views branch 2 times, most recently from 1ec4f70 to 30c9263 Compare June 7, 2024 17:15

bowenyang007 changed the base branch from main to rtso/royalty June 7, 2024 17:15

bowenyang007 force-pushed the rtso/royalty branch from 8b38773 to bbea445 Compare June 7, 2024 17:15

bowenyang007 changed the base branch from rtso/royalty to main June 7, 2024 17:16

bowenyang007 force-pushed the rtso/v1-migration-views branch 7 times, most recently from 27564c8 to f595aba Compare June 10, 2024 21:53

yuunlimm reviewed Jun 11, 2024

View reviewed changes

bowenyang007 and others added 5 commits June 11, 2024 22:31

add v1 views

61e2954

fix move resources view and rebase

1a63149

add missing views

ad1e198

handle royalty and add coin_infos

a2b73c8

fix token query and comment

445628a

bowenyang007 force-pushed the rtso/v1-migration-views branch from cd00178 to 445628a Compare June 12, 2024 05:31

bowenyang007 enabled auto-merge (squash) June 12, 2024 05:31

bowenyang007 merged commit 74c7839 into main Jun 12, 2024
7 checks passed

bowenyang007 deleted the rtso/v1-migration-views branch June 12, 2024 05:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EI-468] Create v1 migration views #385

[EI-468] Create v1 migration views #385

rtso commented May 22, 2024

linear bot commented May 22, 2024

ying-w May 22, 2024

rtso May 23, 2024

ying-w May 31, 2024

bowenyang007 Jun 5, 2024

CapCap May 29, 2024

CapCap May 29, 2024

rtso May 30, 2024

CapCap left a comment

rtso commented May 30, 2024

ying-w May 31, 2024 •

edited

Loading

rtso May 31, 2024

ying-w May 31, 2024

rtso May 31, 2024 •

edited

Loading

ying-w May 31, 2024

ying-w May 31, 2024

rtso May 31, 2024

rtso Jun 3, 2024

bowenyang007 Jun 4, 2024

yuunlimm Jun 5, 2024

yuunlimm Jun 11, 2024

bowenyang007 Jun 12, 2024

		JOIN collections_v2 cv
		ON tdv.collection_id = cv.collection_id AND tdv.transaction_version = cv.transaction_version

[EI-468] Create v1 migration views #385

[EI-468] Create v1 migration views #385

Conversation

rtso commented May 22, 2024

Description

Test plan

linear bot commented May 22, 2024

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

CapCap left a comment

Choose a reason for hiding this comment

rtso commented May 30, 2024

ying-w May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtso May 31, 2024 • edited Loading

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

ying-w May 31, 2024 •

edited

Loading

rtso May 31, 2024 •

edited

Loading