[EI-461] Index royalty in royalty tables #381
Conversation
rtso
changed the title
| ); | ||
|
|
||
| CREATE TABLE current_token_royalty ( | ||
| token_data_id VARCHAR(66) NOT NULL, |
There was a problem hiding this comment.
I forgot why we don't have a unique id for the token.. but using hash value alone as primary key can be dangerous since collision may happen.
There was a problem hiding this comment.
That's a good point. Including creator_address in the primary key can help with collision. Let me add that.
There was a problem hiding this comment.
@larry-aptos explained to me how collision can happen, but i forgot the reason cc @grao1991
There was a problem hiding this comment.
hash id is not guaranteed to be unique. It may be a problem when two different entities have the same hash value.
it seems ahash uses 64bit and considering 10million items, we get this
If we consider the number of items(i.e. token) can go up to 10m, we should take the probability of collision into account and add a field to make pk unique, otherwise, we should be fine.
There was a problem hiding this comment.
wait what? these hashes aren't randomly generated though so can collision still happen? I don't think that creator address is necessary b/c it's part of the hash.
There was a problem hiding this comment.
Also the tokens tables don't have this level of protection so we'll end up with issues anyway.
rust/processor/migrations/2024-05-21-221101_add_royalty_v1/up.sql
Outdated
Show resolved
Hide resolved
bowenyang007
force-pushed
the
rtso/royalty
branch
from
8b38773 to
bbea445
Compare
rust/processor/migrations/2024-05-21-221101_add_royalty_v1/up.sql
Outdated
Show resolved
Hide resolved
|
New changes lgtm. Thanks @bowenyang007 ! |
bowenyang007
force-pushed
the
rtso/royalty
branch
from
32c927e to
b498580
Compare
Description
current_token_royalty_v1table (note that v2 royalty requires collection id which isn't currently supported in this PR)token_v2_processorBackfill
Testnet: 0
Mainnet: 0
Test Plan
Mainnet transaction 613858541
