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

remove cw721 claim limit, and upgrade to v2.6.0 #893

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

NoahSaso
Copy link
Member

@NoahSaso NoahSaso commented Oct 31, 2024

this rewrites the NFT claims logic to make it paginate-able, removing the limit on how many outstanding claims can exist simultaneously

it is backwards compatible by introducing a new state map, seamlessly integrating with the legacy NFT claims so that DAOs can migrate without hassle

specific changes:

  • all new claims go through the new claims logic, which stores claims in a Map<(address, token_id) -> release_at> that can be infinitely paginated, thereby removing any limit on outstanding claims
  • when claiming, you can either pass no argument, an empty token_ids list, or a specific set of token_ids. no argument will attempt to claim all outstanding legacy claims. an empty token_ids list attempts to claim all non-legacy claims, and it will simply fail if there are too many for one transaction. and a specific set of token_ids will attempt to claim just those outstanding non-legacy claims. ideally, we are always specifying the exact tokens to claim, and the frontend can automatically batch as necessary
  • querying claims is now paginated, and all legacy claims are included at the beginning every time. the query's pagination (start_after and limit) only apply to the non-legacy claims

@NoahSaso NoahSaso force-pushed the noah/make-nft-claims-paginatable branch 2 times, most recently from 5e6f3b0 to 815717d Compare October 31, 2024 02:15
@NoahSaso NoahSaso force-pushed the noah/make-nft-claims-paginatable branch from 815717d to 04ba37e Compare October 31, 2024 06:36
@NoahSaso NoahSaso force-pushed the noah/make-nft-claims-paginatable branch from daccce4 to ed90960 Compare October 31, 2024 17:17
@NoahSaso NoahSaso changed the title remove cw721 claim limit remove cw721 claim limit, and upgrade to v2.6.0 Oct 31, 2024
Copy link
Contributor

@ismellike ismellike left a comment

Choose a reason for hiding this comment

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

Looks good!

I think it's just missing checks to ensure we don't have claims in both systems. Other suggestions are mainly for clarity and better API.

packages/cw721-controllers/src/nft_claim.rs Outdated Show resolved Hide resolved
packages/cw721-controllers/src/nft_claim.rs Outdated Show resolved Hide resolved
packages/cw721-controllers/src/nft_claim.rs Outdated Show resolved Hide resolved
contracts/voting/dao-voting-cw721-staked/src/contract.rs Outdated Show resolved Hide resolved
contracts/voting/dao-voting-cw721-staked/src/contract.rs Outdated Show resolved Hide resolved
contracts/voting/dao-voting-cw721-staked/src/contract.rs Outdated Show resolved Hide resolved
@NoahSaso NoahSaso force-pushed the noah/make-nft-claims-paginatable branch from ec6172e to 195da14 Compare October 31, 2024 22:46
Copy link
Contributor

@ismellike ismellike left a comment

Choose a reason for hiding this comment

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

🔥

@NoahSaso NoahSaso merged commit 78226e5 into development Nov 1, 2024
5 of 7 checks passed
@NoahSaso NoahSaso deleted the noah/make-nft-claims-paginatable branch November 1, 2024 18:04
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