From 6faf63ff9597926f855263008014acfd4c349e5e Mon Sep 17 00:00:00 2001 From: kien-ngo Date: Tue, 15 Oct 2024 01:54:28 +0000 Subject: [PATCH] [SDK] Fix upload reveal batch (#5009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem solved Short description of the bug fixed or feature added --- ## PR-Codex overview This PR focuses on fixing the upload logic for creating delayed reveal batches in the `thirdweb` package. It improves the handling of file uploads and the calculation of starting file numbers for batches, ensuring proper functionality during NFT batch creation and reveal. ### Detailed summary - Updated `createDelayedRevealBatch` to use `Promise.all` for concurrent uploads and token ID retrieval. - Added logic to calculate the starting file number for batch uploads. - Removed mock implementations for certain functions in the test file. - Enhanced test cases to verify proper creation and revealing of NFT batches. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` --- .changeset/sweet-pears-change.md | 5 + .../write/createDelayedRevealBatch.test.ts | 155 +++++++++--------- .../write/createDelayedRevealBatch.ts | 22 ++- 3 files changed, 98 insertions(+), 84 deletions(-) create mode 100644 .changeset/sweet-pears-change.md diff --git a/.changeset/sweet-pears-change.md b/.changeset/sweet-pears-change.md new file mode 100644 index 00000000000..fdeea1302bd --- /dev/null +++ b/.changeset/sweet-pears-change.md @@ -0,0 +1,5 @@ +--- +"thirdweb": patch +--- + +Fix upload logic for delayed reveal batch diff --git a/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.test.ts b/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.test.ts index 5712157940d..f7835827456 100644 --- a/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.test.ts +++ b/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.test.ts @@ -1,97 +1,92 @@ -import { beforeAll, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; +import { TEST_CONTRACT_URI } from "~test/ipfs-uris.js"; +import { TEST_ACCOUNT_D } from "~test/test-wallets.js"; import { ANVIL_CHAIN } from "../../../../../test/src/chains.js"; import { TEST_CLIENT } from "../../../../../test/src/test-clients.js"; -import { - type ThirdwebContract, - getContract, -} from "../../../../contract/contract.js"; -import type { Hex } from "../../../../utils/encoding/hex.js"; +import { getContract } from "../../../../contract/contract.js"; +import { deployERC721Contract } from "../../../../extensions/prebuilts/deploy-erc721.js"; +import { sendAndConfirmTransaction } from "../../../../transaction/actions/send-and-confirm-transaction.js"; +import { getNFT } from "../../read/getNFT.js"; import { createDelayedRevealBatch } from "./createDelayedRevealBatch.js"; +import { reveal } from "./reveal.js"; -const mocks = vi.hoisted(() => ({ - getBaseUriFromBatch: vi.fn(), - getBaseURICount: vi.fn(), - encryptDecrypt: vi.fn(), - lazyMint: vi.fn(), - upload: vi.fn(), -})); - -vi.mock("../../../../utils/ipfs.js", () => ({ - getBaseUriFromBatch: mocks.getBaseUriFromBatch, -})); - -vi.mock( - "../../__generated__/IBatchMintMetadata/read/getBaseURICount.js", - () => ({ - getBaseURICount: mocks.getBaseURICount, - }), -); - -vi.mock("../../__generated__/IDelayedReveal/read/encryptDecrypt.js", () => ({ - encryptDecrypt: mocks.encryptDecrypt, -})); - -vi.mock("../../../../storage/upload.js", () => ({ - upload: mocks.upload, -})); - -const placeholderNFT = { +const placeholderMetadata = { name: "Hidden NFT", description: "Will be revealed next week!", }; +const account = TEST_ACCOUNT_D; +const chain = ANVIL_CHAIN; +const client = TEST_CLIENT; +const password = "1234"; -const realNFTs = [ - { - name: "Common NFT #1", - description: "Common NFT, one of many.", - }, - { - name: "Super Rare NFT #2", - description: "You got a Super Rare NFT!", - }, -]; +describe.runIf(process.env.TW_SECRET_KEY)("createDelayedRevealedBatch", () => { + it("should create delayed-reveal batches properly", async () => { + const contract = getContract({ + address: await deployERC721Contract({ + account, + chain, + client, + type: "DropERC721", + params: { + name: "nftdrop", + contractURI: TEST_CONTRACT_URI, + }, + }), + chain, + client, + }); -describe("createDelayedRevealedBatch", () => { - let contract: ThirdwebContract; - beforeAll(() => { - vi.clearAllMocks(); - contract = getContract({ - chain: ANVIL_CHAIN, - address: "0x708781BAE850faA490cB5b5b16b4687Ec0A8D65D", - client: TEST_CLIENT, + // Create batch #0 + await sendAndConfirmTransaction({ + account, + transaction: createDelayedRevealBatch({ + contract, + placeholderMetadata, + password: "1234", + metadata: [{ name: "token 0" }, { name: "token 1" }], + }), }); - }); - it("should generate the proper calldata", async () => { - mocks.getBaseUriFromBatch - .mockReturnValueOnce( - "ipfs://QmQbqDu4aT7sMJHNUk76s4F6DgGk2hVYXYSqpsTRoRM5G8/", - ) - .mockReturnValueOnce( - "ipfs://QmRhASFGXNRE3NXNTfakz82j4Tmv5A9rBezTKGZ5DL6uip/", - ); - mocks.getBaseURICount.mockResolvedValue(0n); - mocks.encryptDecrypt.mockResolvedValue( - "0x8967ae24bd1c6439791bc1c8ca3b3499537283b71af366693792a707eb99e80bc0058c90c1f92f18ec716e4760fdf9279241d442b5b5", - ); + // Create batch #1 + await sendAndConfirmTransaction({ + account, + transaction: createDelayedRevealBatch({ + contract, + placeholderMetadata, + password, + metadata: [{ name: "token 2" }, { name: "token 3" }], + }), + }); - const tx = createDelayedRevealBatch({ - contract, - placeholderMetadata: placeholderNFT, - metadata: realNFTs, - password: "password123", + // Reveal batch #0 + await sendAndConfirmTransaction({ + account, + transaction: reveal({ contract, batchId: 0n, password }), + }); + // Reveal batch #1 + await sendAndConfirmTransaction({ + account, + transaction: reveal({ contract, batchId: 1n, password }), }); - let data: Hex; - if (typeof tx.data === "string") { - data = tx.data; - } else { - data = (await tx.data?.()) || "0x"; - } + /** + * The token URIs of batch 0 should end with "/0" and "/1" + * while the token URIs of batch 1 should end with "/2" and "/3" + */ + const [token0, token1, token2, token3] = await Promise.all([ + getNFT({ contract, tokenId: 0n }), + getNFT({ contract, tokenId: 1n }), + getNFT({ contract, tokenId: 2n }), + getNFT({ contract, tokenId: 3n }), + ]); - expect(data).toEqual( - "0xd37c353b0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000036697066733a2f2f516d516271447534615437734d4a484e556b3736733446364467476b3268565958595371707354526f524d3547382f0000000000000000000000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000040963325533d81f66ae93aff4f562735814e51020ea8bf2bd48eb983ae88cbc40b00000000000000000000000000000000000000000000000000000000000000368967ae24bd1c6439791bc1c8ca3b3499537283b71af366693792a707eb99e80bc0058c90c1f92f18ec716e4760fdf9279241d442b5b500000000000000000000", - ); - expect(mocks.upload).toHaveBeenCalledTimes(2); + expect(token0.tokenURI.endsWith("/0")).toBe(true); + expect(token0.metadata.name).toBe("token 0"); + expect(token1.tokenURI.endsWith("/1")).toBe(true); + expect(token1.metadata.name).toBe("token 1"); + expect(token2.tokenURI.endsWith("/2")).toBe(true); + expect(token2.metadata.name).toBe("token 2"); + expect(token3.tokenURI.endsWith("/3")).toBe(true); + expect(token3.metadata.name).toBe("token 3"); }); }); diff --git a/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.ts b/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.ts index 1389da3141c..05155a29d90 100644 --- a/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.ts +++ b/packages/thirdweb/src/extensions/erc721/lazyMinting/write/createDelayedRevealBatch.ts @@ -14,6 +14,7 @@ import { encryptDecrypt, isEncryptDecryptSupported, } from "../../__generated__/IDelayedReveal/read/encryptDecrypt.js"; +import { nextTokenIdToMint } from "../../__generated__/IERC721Enumerable/read/nextTokenIdToMint.js"; import { lazyMint as generatedLazyMint, isLazyMintSupported, @@ -80,16 +81,29 @@ export function createDelayedRevealBatch( return generatedLazyMint({ contract: options.contract, asyncParams: async () => { - const placeholderUris = await upload({ - client: options.contract.client, - files: Array(options.metadata.length).fill(options.placeholderMetadata), - }); + const [placeholderUris, startFileNumber] = await Promise.all([ + upload({ + client: options.contract.client, + files: Array(options.metadata.length).fill( + options.placeholderMetadata, + ), + }), + nextTokenIdToMint({ + contract: options.contract, + }), + ]); const placeholderUri = getBaseUriFromBatch(placeholderUris); const uris = await upload({ client: options.contract.client, files: options.metadata, + // IMPORTANT: File number has to be calculated properly otherwise the whole batch will break + // e.g: If you are uploading a second batch, the file name should never start from `0` + rewriteFileNames: { + fileStartNumber: Number(startFileNumber), + }, }); + const baseUri = getBaseUriFromBatch(uris); const baseUriId = await getBaseURICount({ contract: options.contract,