0xSooki / erc721-rarity-generator

Generates rarity ranking based off a contract address, using Alchemy
MIT License
3 stars 4 forks source link

Insert all records in one go #10

Closed fekete965 closed 1 year ago

fekete965 commented 1 year ago

Prerequisites

Please answer the following questions for yourself before submitting an issue.

Why are you requesting this feature At the moment when we save the data to the DB we do it one by one, which in my opinion is not very efficient.

Describe the solution you'd like We should prepare all the records first, then use .insertMany to insert all the records into the DB in one go.

fekete965 commented 1 year ago

I would love to solve this problem myself.

0xSooki commented 1 year ago

Amazing, I love your enthusiasm :)

fekete965 commented 1 year ago

@0xSooki sorry for the silly question, but how has the right to push to the develop branch?

0xSooki commented 1 year ago

I will be merging the prs to the develop branch

fekete965 commented 1 year ago

I understand that :) but can anybody create PRs locally originated in that branch?

0xSooki commented 1 year ago

I believe so, but wouldn't bet on it

fekete965 commented 1 year ago

@0xSooki I believe that the forked repos will not see the develop branch. 😞

0xSooki commented 1 year ago

When forking a repo it asks if you only want to work main branch can't that be the case?

fekete965 commented 1 year ago

@0xSooki you are right! I had to add it manually. 😅

0xSooki commented 1 year ago

Everything working perfectly?

fekete965 commented 1 year ago

Yes. Everything is fine now. Refactor PR has been rebased on develop and conflicts were addressed. I am working on inserting all records into the DB atm.

0xSooki commented 1 year ago

Will test it soon on develop than merging it to main

0xSooki commented 1 year ago

Started testing, however the trait values are nulls, any idea why? image

fekete965 commented 1 year ago

@0xSooki I'll start investigating it.

fekete965 commented 1 year ago

@0xSooki it seems like a valid value to me based on the original algorithm. 🤔 I'll double-check the data received and the algorithm itself.

fekete965 commented 1 year ago

@0xSooki we add these values manually here:

if (currentMeta.length < collectionAttributes.length) {
      const attributes = currentMeta.map((e) => e.trait_type);
      const absent = collectionAttributes.filter((e) => !attributes.includes(e));

      absent.forEach((type) => {
        const rarityScoreNull = 1 / ((totalMetadata - tally[type].occurences) / totalMetadata);

        currentMeta.push({
          trait_type: type,
          value: null,
          rarityScore: roundToHundredth(rarityScoreNull)
        });

        totalRarity += rarityScoreNull;
      });
    }
0xSooki commented 1 year ago

So the data fetched from Alchemy does not get assigned?

fekete965 commented 1 year ago

I believe first, we collect all the attributes. Then we compare each nft's attribute list toall of the attributes we have gathered. If a property is "missing" from the NFT's attributes, we will add it with a value of null.

0xSooki commented 1 year ago

Should work something like that, but something seems to be assigning null to every trait value, will investigate it further tonight

fekete965 commented 1 year ago

I'll take a look at the saved records and compare them with the metadata as well today.

0xSooki commented 1 year ago

Would appreciate that

fekete965 commented 1 year ago

@0xSooki I did a little experiment. I have saved all gathered NFT's metadata into its own file. Since we also have the IDs of each element, that made it very easy to find the correct json file related to the NFT.

In the pictures below you will see NFT-335. On the left-hand side, you can see all the attributes that this particular NFT has, which is only a single attribute. On the right-hand side, you can see that we populate the rest of the NFT's attributes with a null value.

As I mentioned it above, I believe this is the algorithm's expected behaviour, but please let me know if this should be something different, we might need to take a look at how we process the data. :)

Screenshot 2022-10-05 at 22 30 41 Screenshot 2022-10-05 at 22 30 47
0xSooki commented 1 year ago

The algorithm is calculating the rarity data perfectly, however in the attributes the value of the trait_type should be present & not null. For example:

{
"trait_type": "Hat",
"value": "Cap",
"rarityScore": 7.24
}
fekete965 commented 1 year ago

Why shouldn't the value be null if that NFT itself doesn't have an attribute like that? 🤔

0xSooki commented 1 year ago

Then it should be, sorry I miss understood, but for some reason for me there were only nulls for values

fekete965 commented 1 year ago

Can an NFT exist without any attributes? (I have a minimal knowledge regarding NFTs, sorry)

0xSooki commented 1 year ago

It's okay, for ERC-721 token standard not really because the token standard is:

{
  "description": "Friendly OpenSea Creature that enjoys long swims in the ocean.",
  "external_url": "https://openseacreatures.io/3",
  "image": "https://storage.googleapis.com/opensea-prod.appspot.com/puffs/3.png",
  "name": "Dave Starbelly",
  "attributes": [
    {
      "trait_type": "Base",
      "value": "Starfish"
    },
    ...
  ]
}

There can be cases when the value is null, when the given token does not have that specific trait but other than that in most cases it should have a value

fekete965 commented 1 year ago

Alright. Thanks for explaining it to me!! 🙏 ❤️

0xSooki commented 1 year ago

After the null issue is fixed we can merge develop into main

fekete965 commented 1 year ago

I will keep investigating the issue when I have time.

0xSooki commented 1 year ago

Much appreciated

fekete965 commented 1 year ago

@0xSooki would you be able to send me your data set? I cannot reproduce this issue anymore. :/

I did loads of testing and I never had an empty attribute list. 😞

0xSooki commented 1 year ago

Not the attribute list itself but the values image

fekete965 commented 1 year ago

@0xSooki then I failed to understand the problem 😞 Everything looks correct to me.

If you don't want attributes with null values, we simply remove the push from the code below:

if (currentMeta.length < collectionAttributes.length) {
      const attributes = currentMeta.map((e) => e.trait_type);
      const absent = collectionAttributes.filter((e) => !attributes.includes(e));

      absent.forEach((type) => {
        const rarityScoreNull = 1 / ((totalMetadata - tally[type].occurences) / totalMetadata);

        currentMeta.push({
          trait_type: type,
          value: null,
          rarityScore: roundToHundredth(rarityScoreNull)
        });

        totalRarity += rarityScoreNull;
      });
    }

We gather every attribute from our dataset:

ALL ATTRIBUTES
Set(11) {
  'Class',
  'Background',
  'Skin',
  'Antenna',
  'Mouth',
  'Eyes',
  'Clothes',
  'Mask',
  'Pet',
  'Special Set',
  'Stamina'
}

Afterwards, we compare the NFT's attribute list with the list above.

// NFT's attribute list
Set(10) {
  'Class',
  'Background',
  'Skin',
  'Antenna',
  'Mouth',
  'Eyes',
  'Clothes',
  'Mask',
  'Pet',
  'Stamina'
}

In this list above, we are missing Special Set, so in our loop, we add this element to the NFT's attribute list and assign a null to it.

We can simply remove the push if you want to avoid having attributes with the value of null.

0xSooki commented 1 year ago

Yupp, this works perfectly & just realised my problem might be with alchemy itself as now it doesen't even work on the backup, will try to write a detailed issue soon about what's in my mind