-
Notifications
You must be signed in to change notification settings - Fork 71
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
add parquet_token_v2_processor #462
Conversation
c1793b4
to
7f41600
Compare
pub mutable_uri: Option<bool>, | ||
pub table_handle_v1: Option<String>, | ||
pub token_standard: String, | ||
#[allocative(skip)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we used to also have a collection_properties
field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
collections is not going to be migrated over in this phase. we can ignore this file for now
pub token_properties: String, | ||
pub description: String, | ||
pub token_standard: String, | ||
pub is_fungible_v2: Option<bool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rtso what's the diff between token_standard and is_fungible_v2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
token_standard
is whether the token is v1 or v2. is_fungible_v2
is whether this token is also fungible (it's an implementation detail based on the contract)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some feedback for
- collections
- token_data
- token_ownerships
did you already move collection_data?
collection_data wasn't a part of parquet. do we need in bq? |
7f41600
to
bc247ca
Compare
bc247ca
to
684b30f
Compare
|
a83c973
to
26370b8
Compare
26370b8
to
206e088
Compare
property_version_v1: 0, | ||
owner_address: Some(owner_address.clone()), | ||
storage_id: storage_id.clone(), | ||
amount: "0".to_string(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: make 0
as constant, it's hard to understand why this is 0 without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good. will also add a comment
// set to negative of event index to avoid collison with write set index | ||
write_set_change_index: -1 * event_index, | ||
token_data_id: token_data_id.clone(), | ||
property_version_v1: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like LEGACY_DEFAULT_PROPERTY_VERSION
would help.. btw, is it due to v1 migration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not quite sure if it was added as a part of migration.
d189d84
to
4f55d77
Compare
Description
Add parquet token processor that is responsible for
1. collection_v2this will be omitted for now.2. token_datas_v2
3. token_ownerships_v2
Test Plan
token_ownerships
collection_v2