Add missing fields #110
Add missing fields #110
Conversation
…i/stick into add-mssing-fields
db/migrations/1689623692588-Data.js
Outdated
There was a problem hiding this comment.
Hello, thanks for the implementation I see that you tried to do your best!
I have a few main issues:
1. Code structure
as we spoke before I try to keep some code structure.
I suggest create src/mappings/shared/tokens
and create file per each handler: create.ts, list.ts etc.
2. you forget awaits
eventHandlers.listHandler(context, entity) will make the indexer fail in the next block because connection will still locked by this
3. too much repetitive code
I see that some simple functions are often repeated a lot.
When someone decides to refactor the code. he/she can fail so this simple code functions.
4. missing buy handler
when someone buys the cheapest NFT the token will not be reflected.
5. the resolver will kill the squid
FOUR joins will make the server cry.
Otherwise
very good job!
| FROM | ||
| token_entity as t | ||
| LEFT JOIN nft_entity as ne ON t.id = ne.token_id AND ne.current_owner = $1 | ||
| LEFT JOIN CheapestNFT as c ON t.id = c.token_id | ||
| LEFT JOIN collection_entity as col ON t.collection_id = col.id | ||
| LEFT JOIN metadata_entity as me ON t.meta_id = me.id |
There was a problem hiding this comment.
4 left joins is not a performance wise
There was a problem hiding this comment.
I have benchmarked a few queries, some with 3 and some with 4 joins
tested by a bash script that runs an SQL query 200 times against the fully loaded DB on localhost
results are, with no significant difference between the approaches
mean between 1.55 to 1.65 ms
standard deviation ~0.25 ms
|
Hi @vikiival let's address the points: 1, 2, 4 sure, np, will fix
mind point the finger at where?
I understand
WITH CheapestNFT AS (
SELECT
ne.token_id,
MIN(ne.price) AS "cheapestNFTPrice",
COUNT(ne.id) as count
FROM
nft_entity ne
WHERE
ne.current_owner = $1
GROUP BY
ne.token_id
)
SELECT
t.id as id,
t.name as name,
t.image as image,
t.media as media,
t.metadata as metadata,
me.id as "metaId",
me.description as "metaDescription",
me.image as "metaImage",
me.animation_url as "metaAnimationUrl",
t.block_number as "blockNumber",
t.created_at AS "createdAt",
t.updated_at AS "updatedAt",
c.count as count,
c."cheapestNFTPrice",
col.id AS "collectionId",
col.name AS "collectionName"
FROM
token_entity as t
JOIN CheapestNFT as c ON t.id = c.token_id
JOIN collection_entity as col ON t.collection_id = col.id
JOIN metadata_entity as me ON t.meta_id = me.id
WHERE
($5::bigint IS NULL OR c."cheapestNFTPrice" >= $5::bigint) AND
($6::bigint IS NULL OR c."cheapestNFTPrice" > $6::bigint) AND
($7::bigint IS NULL OR c."cheapestNFTPrice" <= $7::bigint)
ORDER BY $4 LIMIT $2 OFFSET $3; |
|
|
all done @vikiival please check again |
| } | ||
| const tokenId = generateTokenId(collection.id, nftMedia) | ||
| debug(OPERATION, { createToken: `Create TOKEN ${tokenId} for NFT ${nft.id}` }) | ||
| const tokenName = typeof nft.name === 'string' ? nft.name?.replace(/([#_]\d+$)/g, '').trim() : '' |
|
@vikiival fixed utils issue also, now i realize, the usage from FE will have to be via the resolver only
|
The scary one? 🥺 |
indeed owner filtering => count/supply change => resolver it's not so scary. just join on collection and metadata and 2 CTE's |
have you ran some sample tests? how it behaves on the frontend? |
yes , I am currently integrating with FE running the squid on localhost so far it works very well also, as mentioned here #110 (comment) |
|
work in progress:
|
a. dropped it from schmea as it is problematic (but and but events of chepeast will require fetching all token's nft's to find the cheapest again)
dropped, replaced by cheapest
done |
|
@vikiival let me know if you still want me to change the handlers names Thank you :) |
Like this is ok |
fork from https://github.com/daiagi/stick/tree/tokenEnityt/fix-count-calc
implement kodadot/loligo#51 for stick
specifically,