-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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 #3945
Remove Address.isContract #3945
Conversation
contracts/utils/Address.sol
Outdated
* [IMPORTANT] | ||
* ==== | ||
* It is unsafe to assume that an address for which this function returns | ||
* false is an externally-owned account (EOA) and not a contract. | ||
* | ||
* Among others, `isContract` will return false for the following | ||
* types of addresses: | ||
* | ||
* - an externally-owned account | ||
* - a contract in construction | ||
* - an address where a contract will be created | ||
* - an address where a contract lived, but was destroyed | ||
* ==== | ||
* | ||
* [IMPORTANT] | ||
* ==== | ||
* You shouldn't rely on `isContract` to protect against flash loan attacks! | ||
* | ||
* Preventing calls from contracts is highly discouraged. It breaks composability, breaks support for smart wallets | ||
* like Gnosis Safe, and does not provide security since it can be circumvented by calling from a contract | ||
* constructor. | ||
* ==== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create a new file at docs/modules/ROOT/pages/knowledge.adoc
, add it in docs/modules/ROOT/nav.adoc
, and include this content there so we don't lose it. I think it should be a section called "Can I restrict a function to EOAs only?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the new doc page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of "FAQ" as the title for the section? Or you can check how I did it and if it can remain like that, no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes FAQ works.
Co-authored-by: Francisco <[email protected]>
Co-authored-by: Francisco <[email protected]>
Co-authored-by: Francisco <[email protected]>
@@ -0,0 +1,14 @@ | |||
Can I restrict a function to EOAs only? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The answer to this question is closer to "no", but the content below seems to be saying "yes".
The key part that is missing is that although isContract
(equivalently, addr.code.length > 0
) may seem to differentiate contracts from EOAs, it can only say that an address is currently a contract, and its negation (that an address is not currently a contract) does not imply that the address is an EOA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, please check the new version to see if its clear enough after the update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docs/modules/ROOT/pages/faq.adoc
Outdated
|
||
When calling external addresses from your contract it is unsafe to assume that an address is an externally-owned account (EOA) and not a contract. Preventing calls from contracts is highly discouraged. It breaks composability, breaks support for smart wallets like Gnosis Safe, and does not provide security since it can be circumvented by calling from a contract constructor. | ||
|
||
Some criteria to discard an address as a contract address can be: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an address has code size = 0, this does not mean that it is not a contract. Some counterexamples are:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the updated version
🦋 Changeset detectedLatest commit: 91802e5 The changes in this PR will be included in the next version bump. 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 |
Replaces #3682
Fixes #3417
PR Checklist