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

Add Foundry test for failures when spending pre-approved items from null address with invalid signature #502

Merged
merged 3 commits into from
Jun 11, 2022

Conversation

emo-eth
Copy link
Contributor

@emo-eth emo-eth commented Jun 10, 2022

Test that signature verification works in the case where Seaport has permission to transfer an item at the null address, and fails accordingly.

James Wenzel added 2 commits June 10, 2022 14:17
…ix unused param in _performTestFulfillOrderRevertInvalidArrayLength
@emo-eth emo-eth requested review from stuckinaboot and 0age June 10, 2022 22:15
Copy link
Contributor

@0age 0age left a comment

Choose a reason for hiding this comment

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

Can we change the phrasing to make it clear that this is testing for failure when attempting this? Only other thing that we might want to tweak is to use the solmate token already available as a library rather than adding the token as a new contract. Let me know if there’s some detail I’m missing though. (Everything else looks solid!)

@emo-eth
Copy link
Contributor Author

emo-eth commented Jun 11, 2022

@0age can definitely make language clearer. Solmate tokens don't have virtual modifiers on internal methods, and use the generated public property getters for ie isApprovedForAll, which can't be overridden either. I opted to modify Solmate's 721 rather than add openzeppelin-contracts as a Foundry dependency, but we can do that instead if you'd prefer.

@emo-eth emo-eth changed the title Add Foundry test for spending pre-approved items from null address with invalid signature Add Foundry test for failures when spending pre-approved items from null address with invalid signature Jun 11, 2022
@0age 0age merged commit 06bf918 into main Jun 11, 2022
@0age 0age deleted the test-null-spend branch June 11, 2022 02:58
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