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

Deprecation warning for SELFDESTRUCT #13844

Closed
ekpyron opened this issue Jan 4, 2023 · 11 comments · Fixed by #13884
Closed

Deprecation warning for SELFDESTRUCT #13844

ekpyron opened this issue Jan 4, 2023 · 11 comments · Fixed by #13884
Assignees
Labels
low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. selected for development It's on our short-term development
Milestone

Comments

@ekpyron
Copy link
Member

ekpyron commented Jan 4, 2023

Following https://eips.ethereum.org/EIPS/eip-6049 we should emit deprecation warnings on any use of selfdestruct.

@ekpyron ekpyron added selected for development It's on our short-term development low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. labels Jan 4, 2023
@ekpyron ekpyron added this to the 0.8.18 milestone Jan 4, 2023
@morganjweaver
Copy link

morganjweaver commented Jan 11, 2023

Hello there. I'm a newer Solidity dev with a C++ and compilers background. I've found the Contributing to Solidity 101 doc, the Workflow for PRs doc, and the PR Review Checklist doc, and intend to submit a PR. Please let me know if there is anything not covered in the documentation that I should know. I've also got the gitter solidity-dev channel open. Thank you!

Also: since the callcode deprecation warning is most similar, I wanted to verify that I should include inline assembly in the check + warning, unlike callcode--is this correct?

@ekpyron
Copy link
Member Author

ekpyron commented Jan 11, 2023

Hi there and thank you very much for wanting to contribute!

Yes, callcode may be a good reference for emitting the deprecation warning on the solidity level - that was done a long time ago and the code base has changed a lot since, but the StaticAnalyzer should still work pretty much the same.

And indeed there should also be a warning about selfdestruct in inline-assembly, resp. Yul - the place to look for that would be libyul/AsmAnalysis.cpp - but we can also start with a PR that only addresses Solidity and take it from there!

Feel free to ask for any help you may need here or in the gitter/matrix channel - unfortunately, it seems like we're in different timezones, so we may have to rely on asynchronous communication.

I hope https://docs.soliditylang.org/en/develop/contributing.html will be a good enough reference to get you started e.g. on running the test suite. https://docs.soliditylang.org/en/develop/contributing.html#writing-and-running-syntax-tests and running isoltest should be the most relevant for this issue.

@ekpyron
Copy link
Member Author

ekpyron commented Jan 16, 2023

@morganjweaver Any update on this? Do you need any more input and/or help?

@nikola-matic
Copy link
Collaborator

@morganjweaver I'll be taking this over since we plan on releasing soon, and we'd rather like for this to go into the 0.8.18 release.

@cameel
Copy link
Member

cameel commented Jan 19, 2023

And indeed there should also be a warning about selfdestruct in inline-assembly

Maybe it would be better not to issue a warning for its use in inline assembly? At the moment the opcode is still in the EVM so if you need and want its functionality, using selfdestruct() is still the way to go. There is no alternative construct available now and there's no way to silence this warning.

@nikola-matic
Copy link
Collaborator

And indeed there should also be a warning about selfdestruct in inline-assembly

Maybe it would be better not to issue a warning for its use in inline assembly? At the moment the opcode is still in the EVM so if you need and want its functionality, using selfdestruct() is still the way to go. There is no alternative construct available now and there's no way to silence this warning.

The draft PR only issues a warning when used from Solidity, so we're good so far if that's the case.

@cameel
Copy link
Member

cameel commented Jan 19, 2023

Yeah, I saw that. That's good for now but it was stated here that we should add it in inline assembly too so I wanted to get a decision on that.

@ekpyron
Copy link
Member Author

ekpyron commented Jan 23, 2023

Definitely also warn in assembly. The opcode is what's being deprecated.

@trendespresso
Copy link

Definitely also warn in assembly. The opcode is what's being deprecated.

As an EVM developer a little out of the know: What should I replace selfdestruct() with? In the case of a payable() inside, how should that look: selfdestruct(payable(msg.sender)) --> ??

Thanks for any info! Appreciate everyone who works on Solidity
Cheers

@trendespresso
Copy link

If we want to destroy contracts, how should we do so now that selfdestruct() is deprecated?

@Tshembani04
Copy link

I used solidity: "0.8.17", after trying to get anything resourceful from the Ethereum docs and chatGPT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. selected for development It's on our short-term development
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants