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

feat(foundation): add support for ERC1155 mints #445

Merged
merged 12 commits into from
Jun 11, 2024
Merged

Conversation

mmackz
Copy link
Collaborator

@mmackz mmackz commented Jun 11, 2024

Refactors the foundation plugin to add support for ERC-1155 mints.

Notes

For 1155 tokens, this implementation requires a tokenId or it will throw an error. Normally we target the initial tokenId in the collection, but this does not work here because the tokenIds are not sequential and can be any random number.

Here are the relative contracts:

@mmackz mmackz requested a review from a team as a code owner June 11, 2024 05:04
Copy link

changeset-bot bot commented Jun 11, 2024

🦋 Changeset detected

Latest commit: 5cf124d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@rabbitholegg/questdk-plugin-foundation Minor
@rabbitholegg/questdk-plugin-registry Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@Quazia Quazia left a comment

Choose a reason for hiding this comment

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

This looks good - but as the code exists it's going to cause issues with the way the backend implements fees.

Here's where it gets the fees

That value then gets saved into the quest object in the DB

Reading through this code the Dutch Auction fees are going to change, so the front-end embedded mint is going to need to look up the fees pre-flight, not rely on the cached values. Potentially requires backend and front-end changes to support the dynamic fees.

return amount
}

export function getMintAmount(amount: FilterOperator | undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

This might actually be worth putting in utils - seems useful for most simulations

@mmackz
Copy link
Collaborator Author

mmackz commented Jun 11, 2024

This looks good - but as the code exists it's going to cause issues with the way the backend implements fees.

Here's where it gets the fees

That value then gets saved into the quest object in the DB

Reading through this code the Dutch Auction fees are going to change, so the front-end embedded mint is going to need to look up the fees pre-flight, not rely on the cached values. Potentially requires backend and front-end changes to support the dynamic fees.

Thankfully the dutch auction fee type is very rarely used. Good callout, the fees will definitely increase after a few mints are made.

What is the best way to deal with this in the short term? Should we just throw an error and prevent people from launching boosts with dutch auction?

@Quazia
Copy link
Member

Quazia commented Jun 11, 2024

I'd say prevent it for now until we can modify it in the front-end and back-end if it's not used high frequency.

@mmackz mmackz merged commit 98ddd4a into main Jun 11, 2024
9 checks passed
@mmackz mmackz deleted the mmackz/foundation-1155 branch July 2, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants