Skip to content

Conversation

@easonchai
Copy link
Contributor

@easonchai easonchai commented Nov 27, 2022

This is a patch for the ERC1155 Signature Mint function in the SDK.

The current function uses the domain TokenERC1155 which should actually match the domain in the smart contract, which is SignatureMintERC1155. Because the domains do not match, the signer recovered after processing the request will always be different. This means all transactions will fail as the signer recovered is not expected.

This change was also communicated and discussed with @adam-maj on Discord after a week of debugging.

Changes were tested according to contributing guidelines!

@changeset-bot
Copy link

changeset-bot bot commented Nov 27, 2022

🦋 Changeset detected

Latest commit: 825a919

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

This PR includes changesets to release 4 packages
Name Type
@thirdweb-dev/sdk Patch
@thirdweb-dev/auth Patch
thirdweb Patch
@thirdweb-dev/react 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

@joaquim-verges
Copy link
Member

@easonchai thank you for this! great catch.

We still need to handle the case for our own Edition contract which uses TokenERC1155 as the domain, i've added a case for this. Also added a test for contracts using the SignatureMintERC1155 ContractKit extension.

If all tests pass we should be good to merge :)

Thank you again!

@joaquim-verges joaquim-verges enabled auto-merge (squash) November 28, 2022 02:55
@joaquim-verges joaquim-verges merged commit f03be39 into thirdweb-dev:main Nov 28, 2022
@github-actions github-actions bot mentioned this pull request Nov 28, 2022
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