Optional NFT Types #90
Conversation
|
@whilefoo rfc |
|
@0x4007 why did we want to make them optional in the first place? |
|
It isn't required for ERC20 Permits #69 (comment) |
|
@luisantoniocrag, this task has been idle for a while. Please provide an update. |
|
Hey! All good? Anything I'm missing? @0x4007 |
| @@ -67,7 +67,8 @@ export async function generateErc721PermitSignature( | |||
| _repositoryName = contextOrPermitPayload.repositoryName; | |||
| _userId = contextOrPermitPayload.userId; | |||
| } else { | |||
| const { NFT_MINTER_PRIVATE_KEY, NFT_CONTRACT_ADDRESS } = contextOrPermitPayload.env; | |||
| const { NFT_MINTER_PRIVATE_KEY = "", NFT_CONTRACT_ADDRESS = "" } = contextOrPermitPayload.env; | |||
There was a problem hiding this comment.
This should throw if these are not defined rather than assigning them an empty string. Otherwise I think things are okay
| if (!_nftContractAddress) { | ||
| const errorMessage = "NFT contract address is not defined"; | ||
| _logger.error(errorMessage); | ||
| throw new Error(errorMessage); | ||
| } | ||
|
|
There was a problem hiding this comment.
This block of code was removed since above a validation is already performed when NFT_CONTRACT_ADDRESS is not defined or its string value is empty
| @@ -117,7 +117,7 @@ describe("generateErc721PermitSignature", () => { | |||
|
|
|||
| it("should throw an error if NFT minter private key is not defined", async () => { | |||
| delete process.env.NFT_MINTER_PRIVATE_KEY; | |||
| await expect(generateErc721PermitSignature(context, "123", "contribution")).rejects.toThrow("Failed to" + " instantiate wallet"); | |||
| await expect(generateErc721PermitSignature(context, "123", "contribution")).rejects.toThrow("NFT minter" + " private key" + " is not defined"); | |||
There was a problem hiding this comment.
I Updated the error message in the unit test when NFT_MINTER_PRIVATE_KEY is not defined with a more concise message in this case
|
Done @Keyrxng 👍 |
|
|
||
| if (!NFT_MINTER_PRIVATE_KEY) { | ||
| _logger.error("NFT minter private key is not defined"); | ||
| throw new Error("NFT minter private key is not defined"); |
There was a problem hiding this comment.
you can put message in variable to avoid duplicating
|
Alright, thanks @whilefoo! done ✅ |
|
@luisantoniocrag, this task has been idle for a while. Please provide an update. |
Resolves #88