Skip to content
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

Fix/simple-mint #6380

Merged
merged 10 commits into from
Jul 6, 2023
Merged

Fix/simple-mint #6380

merged 10 commits into from
Jul 6, 2023

Conversation

daiagi
Copy link
Contributor

@daiagi daiagi commented Jul 4, 2023

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Needs QA check

Context

Did your issue had any of the "$" label on it?

Screenshot 📸

  • My fix has changed UI

Copilot Summary

🤖 Generated by Copilot at b2f1e61

Fixed a type bug in the simple minting component and added support for minting NFTs under existing collections by providing the collection ID as a string. Modified the getNftId function in scheme.ts to handle both cases.

🤖 Generated by Copilot at b2f1e61

To mint NFTs with more ease
We added a new feature, please
Check the nft.collection
And its type detection
And fix the rmrkMint.max keys

@daiagi daiagi requested a review from a team as a code owner July 4, 2023 05:39
@daiagi daiagi requested review from preschian and Jarsen136 and removed request for a team July 4, 2023 05:39
@kodabot
Copy link
Collaborator

kodabot commented Jul 4, 2023

WARNING @daiagi PR for issue #6343 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #6343

@netlify
Copy link

netlify bot commented Jul 4, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit a046296
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64a62b0314100f0008e8eee5
😎 Deploy Preview https://deploy-preview-6380--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@reviewpad
Copy link
Contributor

reviewpad bot commented Jul 4, 2023

AI-Generated Summary: This pull request includes two patches. The first patch makes a modification in 'SimpleMint.vue' to ensure that 'this.rmrkMint.max' is assigned as a number to avoid type related errors or inconsistencies. The patch also modifies the 'syncEdition' and 'sub' methods to ensure compatibility with this change. The second patch addresses an issue in 'getNftId' function within 'service/scheme.ts'. The fix is to handle a scenario where the 'collection' property of 'nft' is not an object but a string, by checking its type before using it to construct the 'collectionId'.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Jul 4, 2023
@prury
Copy link
Member

prury commented Jul 4, 2023

First test:
image

tx: https://kusama.subscan.io/tx/0x24af384214522020f7262e4f6059e9b2e05bf338982f899b1d5539dfeae9e8b7
tx: https://kusama.subscan.io/tx/0xc43177b96299c8435b79e9fbcaf9afae80e43662987b4913792b2be23157fa10

4 editions successfully minted:
image

Distribution:

I've set the distribution to 75% via slider, as i understood 3 of 4 NFTs should have been sent to the addresses i had filled, but none got sent.

@daiagi I'll open another issue to fix distribution

@daiagi
Copy link
Contributor Author

daiagi commented Jul 4, 2023

successful mint with 2 editions:
https://kusama.subscan.io/extrinsic/18641451-5
/rmrk/collection/C4F63647002B182C0E-WOLF

successful mint with 2 editions + list for sell:
mint: https://kusama.subscan.io/extrinsic/18641561-3
list: https://kusama.subscan.io/extrinsic/18641565-3
/rmrk/collection/C4F63647002B182C0E-WOLF2

redirect after mint success (with and w/o listing for sell)

bugs for other issues:

  1. no loader animation during the list for sell transaction
  2. I've set the distribution to 75% via slider, as i understood 3 of 4 NFTs should have been sent to the addresses i had filled, but none got sent.

@daiagi daiagi requested a review from vikiival July 4, 2023 13:51
@vikiival
Copy link
Member

vikiival commented Jul 4, 2023

I've set the distribution to 75% via slider, as i understood 3 of 4 NFTs should have been sent to the addresses i had filled, but none got sent.

I am checking the code that I have written around 06/2021

How it works:

  1. we parse all addressed
  2. shuffle them via on-chain randomness
  3. if distributon is set (e.g 75) - from 4 addr we take 3
    4a. if you have less tokens than addresses let's say 2 we will take first 2 addresses
    4b. if you have more tokens (10) we take 3 addresses as stated in step 3
  4. transfer tx is done

If you see some logical error (it does not make sense) we can still change the algo

@prury
Copy link
Member

prury commented Jul 4, 2023

I've set the distribution to 75% via slider, as i understood 3 of 4 NFTs should have been sent to the addresses i had filled, but none got sent.

I am checking the code that I have written around 06/2021

How it works:

1. we parse all addressed

2. shuffle them via on-chain randomness

3. if distributon is set (e.g 75) - from 4 addr we take 3
   4a. if you have less tokens than addresses let's say 2 we will take first 2 addresses
   4b. if you have more tokens (10) we take 3 addresses as stated in step 3

4. transfer tx is done

If you see some logical error (it does not make sense) we can still change the algo

oh, it works like a random draw thing then
the collection I've created had 4 nfts, 3 addresses and a 75% distribution percentage
at least 2 should have been transferred, but no one got an NFT

@prury prury mentioned this pull request Jul 4, 2023
1 task
@vikiival
Copy link
Member

vikiival commented Jul 4, 2023

but no one got an NFT

ah @daiagi I know
In: @components/accounts/utils
line 57
you need to pass different version 💯

@daiagi
Copy link
Contributor Author

daiagi commented Jul 4, 2023

but no one got an NFT

ah @daiagi I know In: @components/accounts/utils line 57 you need to pass different version 💯

@vikiival I removed the version param as per createInteraction signature:

image

@vikiival
Copy link
Member

vikiival commented Jul 4, 2023

I removed the version param as per createInteraction signature:

Oh therefore you need to import also createInteraction from minimark/v2 and if prefix is kusama then use v2

@daiagi
Copy link
Contributor Author

daiagi commented Jul 4, 2023

Oh therefore you need to import also createInteraction from minimark/v2 and if prefix is kusama then use v2

alrighty, to be continued tommorow

@daiagi
Copy link
Contributor Author

daiagi commented Jul 5, 2023

Oh therefore you need to import also createInteraction from minimark/v2 and if prefix is kusama then use v2

done
@prury wanna give it a try?

Copy link
Member

@vikiival vikiival left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be oki now

@prury
Copy link
Member

prury commented Jul 5, 2023

Oh therefore you need to import also createInteraction from minimark/v2 and if prefix is kusama then use v2

done @prury wanna give it a try?

lemme try

@prury
Copy link
Member

prury commented Jul 5, 2023

@prury
Copy link
Member

prury commented Jul 5, 2023

trying now with less copies than addresses:
image

Settings:
2 editions
listing for 0.4
4 addresses
100% distribution

collection:
https://deploy-preview-6380--koda-canary.netlify.app/rmrk/collection/7EA1DCF47E98A25067-IH?collectionId=7EA1DCF47E98A25067-IH

NFTs got sent successfully

https://kusama.subscan.io/tx/0x834d2c358595380661d4395f2c67f271d7066756c02691a7aa2f15fa0b2453e3
https://kusama.subscan.io/tx/0x9265ce9d0f536a5b8d805f0b2f0e25f86ddf3a2110bc12bc1f6e3be94f63cc4f

obs: listing was just a unused extra step because i was transferring them to other accounts

@daiagi is it possible to change Editions to Copies like we did for statemine?

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jul 5, 2023
@vikiival
Copy link
Member

vikiival commented Jul 5, 2023

I got NFT 🥹

@codeclimate
Copy link

codeclimate bot commented Jul 6, 2023

Code Climate has analyzed commit a046296 and detected 0 issues on this pull request.

View more on Code Climate.

@daiagi
Copy link
Contributor Author

daiagi commented Jul 6, 2023

is it possible to change Editions to Copies like we did for statemine?

yes. done

@vikiival
Copy link
Member

vikiival commented Jul 6, 2023

Sooo let's merge this

@vikiival vikiival merged commit f59f101 into main Jul 6, 2023
18 of 19 checks passed
@vikiival vikiival deleted the fix/simple-mint branch July 6, 2023 10:14
@vikiival
Copy link
Member

vikiival commented Jul 6, 2023

pay 100 usd

@yangwao
Copy link
Member

yangwao commented Jul 6, 2023

😍 Perfect, I’ve sent the payout
💵 $100 @ 5.29 USD/DOT ~ 18.904 $DOT
🧗 1cAsKZYNRb8dkSCpn4eVkEn6ycNZTGoDo5dGDgB8J1UUWK8
🔗 0x993cd44ec0d6ead6f671d789c2d21c7ecd7501e64726edd8e17fdf3566774d03

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Jul 6, 2023
This was referenced Jul 17, 2023
@prury prury mentioned this pull request Aug 30, 2023
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paid pull-request has been paid S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix distribution on rmrk/mint Simple Minting on Kusama - Editions not working
7 participants