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

Remove Address.isContract #3682

Conversation

ashwinYardi
Copy link
Contributor

@ashwinYardi ashwinYardi commented Sep 6, 2022

Fixes #3417

  • This PR removes isContract utility function from Address.sol library.
  • Wherever isContract was used in the other contracts, is replaced by inlining address(...).code.length > 0

PR Checklist

  • Tests
  • Documentation
  • Changelog entry

@ashwinYardi ashwinYardi changed the title Deprecate Adddress.isContract Deprecate Address.isContract Sep 6, 2022
@frangio frangio changed the title Deprecate Address.isContract Remove Address.isContract Sep 17, 2022
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thanks!

The pragma in each of these files should be bumped to at least ^0.8.1 because prior to that Solidity would not use extcodesize.

Additionally, this is the removal that we want to do in 5.0, but we should also simply mark it as deprecated in the documentation prior to 5.0.

@@ -21,6 +21,7 @@
### Deprecations

* `EIP712`: Added the file `EIP712.sol` and deprecated `draft-EIP712.sol` since the EIP is no longer a Draft. Developers are encouraged to update their imports. ([#3621](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3621))
* `Address.isContract`: `isContract` utilitiy function is deprecated from v5.0.0 because of its ambiguous nature. Reasons and discussions here: [#3417](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3417).
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put this under a new "Unreleased" section at the top of the file.

@@ -21,6 +21,7 @@
### Deprecations

* `EIP712`: Added the file `EIP712.sol` and deprecated `draft-EIP712.sol` since the EIP is no longer a Draft. Developers are encouraged to update their imports. ([#3621](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3621))
* `Address.isContract`: `isContract` utilitiy function is deprecated from v5.0.0 because of its ambiguous nature. Reasons and discussions here: [#3417](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3417).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `Address.isContract`: `isContract` utilitiy function is deprecated from v5.0.0 because of its ambiguous nature. Reasons and discussions here: [#3417](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3417).
* `Address`: Removed `isContract` because of its ambiguous nature and potential for misuse. ([#3682](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3682))

@JulissaDantes JulissaDantes mentioned this pull request Jan 11, 2023
3 tasks
@JulissaDantes
Copy link
Contributor

This will be closed due to inactivity. This effort will continue on #3945 .

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.

None yet

3 participants