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

Document limitation of delete for EnumerableSet and Map #3400

Closed
frangio opened this issue May 6, 2022 · 4 comments · Fixed by #3412
Closed

Document limitation of delete for EnumerableSet and Map #3400

frangio opened this issue May 6, 2022 · 4 comments · Fixed by #3412
Labels
documentation Inline comments, guides, and examples. good first issue Low hanging fruit for new contributors to get involved!

Comments

@frangio
Copy link
Contributor

frangio commented May 6, 2022

Solidity currently allows executing the delete operator on structs like EnumerableSet and EnumerableMap even though they contain a mapping that will not be cleared, and the operation will in fact corrupt the struct's data.

According to ethereum/solidity#11843 this operation will no longer be allowed in Solidity 0.9, but in the meantime we should document that delete should not be used with these types.

@frangio frangio added good first issue Low hanging fruit for new contributors to get involved! documentation Inline comments, guides, and examples. labels May 6, 2022
@Code-Donewell
Copy link

hi @frangio . I'm new here on git hub(a little overwhelming for me rn 😅) and I recently started learning to make smart contracts via solidity.
pls tell me how can i take on this task . and what exactly I have to do .
Do you mean we have to comment in the smart contracts that 'delete' operator should not be used wherever the related structs has been used in the contracts. Pls tell me how to apply for this particular task 😅.

@frangio
Copy link
Contributor Author

frangio commented May 11, 2022

Yes, in the documentation for these two libraries we have to add a WARNING: ... line saying delete should not be used, it will corrupt the data structure, and the clear function should be used instead.

@lucasAlonso
Copy link
Contributor

Hi, new contributing to OZ!
something like this would be fine?

// WARNING!
// delete function should not be used. It will corrupt the data structure,
// clear function should be used instead.
// ref https://github.com/ethereum/solidity/pull/11843

Also, where i must put it? in the header after introduction or when delete is used?

@SamuelOsewa
Copy link

I believe that it should be when the delete is used so if someone wants to see the use case they would see the comment as people tend to skip over the introduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Inline comments, guides, and examples. good first issue Low hanging fruit for new contributors to get involved!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants