Improve Initializable docstrings

[tinchoabbate]

Updating the docstrings of the Initializable contract so they better reflect the actual behavior of the code.

Changes include:

1. Warning that reinitializer(1) is not equivalent to initializer. Because one allows nesting while the other doesn't. I noticed this was even mentioned in Simplify Initializable #3450, so I guess you're already aware of the difference, and the docs were just outdated.
2. Warning that reinitializer(255) will prevent further reinitializations.
3. Mentioning that these functions emit events.

There's one additional thing I wanted to raise, that I noticed while writing (2) but I did not mention in the docs. At first sight, it may seem that a function that executes reinitializer(255) would behave the same as calling _disableInitializers, in terms of disabling further reinitialization. However, _disableInitializers explicitly requires that the contract is not initializing. While reinitializer(255) first checks the same, then it would continue execution into initializing mode. So it feels like a subtle inconsistency, but I'm not sure to what extent it could actually be problematic, or even worth highlighting in the docs.

[frangio]

Thanks! These are great additions.

I'm not sure I agree with the removal of the "equivalent to..." notes. While you're right that they are not exactly equivalent, and we should definitely document the differences, I think the comparisons are valuable to give an intuition about what the modifiers do and how they relate. Perhaps we should simply replace "equivalent" with "similar", and explain how the differ (as you've done). Do you agree, and can you make this change?

Regarding the last point about the difference between _disableInitializers() and reinitializer(255), if I understand correctly what you're saying is that the modifier will disable and then execute the body of the reinitializer. But as far as I can tell that's the only place where they differ? I see them being consistent in the sense that the disable function could be equivalently written as function _disableInitializers() reinitializer(255) {}.

[tinchoabbate]

I'm not sure I agree with the removal of the "equivalent to..." notes. While you're right that they are not exactly equivalent, and we should definitely document the differences, I think the comparisons are valuable to give an intuition about what the modifiers do and how they relate. Perhaps we should simply replace "equivalent" with "similar", and explain how the differ (as you've done). Do you agree, and can you make this change?

I see your point. Agreed! I changed it so that it better states that the modifiers are indeed similar, except for the mentioned behavior. I also removed the WARNING label for these, because I think they were too strong.

Regarding the last point about the difference between _disableInitializers() and reinitializer(255), if I understand correctly what you're saying is that the modifier will disable and then execute the body of the reinitializer. But as far as I can tell that's the only place where they differ?

Yes, that's my point. Though I might be overthinking it, and probably the discussion is out of scope of the PR. If I can come up with an example scenario to be clearer, I'll open an issue.

[frangio]

Approved with minor changes. Thanks!

[gitpoap-bot]

Woohoo, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2022 OpenZeppelin Contracts Contributor:

Head on over to GitPOAP.io and connect your GitHub account to mint!