-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Group Typo Fixes 4 #5750
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
base: master
Are you sure you want to change the base?
Group Typo Fixes 4 #5750
Conversation
Co-authored-by: Hadrien Croubois <[email protected]>
|
…ts into typo-fixes
…tracts into typo-fixes
Signed-off-by: sellskin <[email protected]> Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Ernesto García <[email protected]>
Co-authored-by: Ernesto García <[email protected]>
Co-authored-by: fifalodm <[email protected]> Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Ernesto García <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: Hadrien Croubois <[email protected]>
WalkthroughEditorial and minor code edits across contracts, docs, scripts, and tests: README token wording and Defender promo removed; import switched to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Co-authored-by: Arr00 <[email protected]>
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
contracts/utils/cryptography/draft-ERC7739Utils.sol (2)
64-66
: Repair broken sentence in NOTE.Second line “data instead.” is dangling; fold into a single clear sentence.
Apply this diff:
- * NOTE: This function returns empty if the input format is invalid instead of reverting. - * data instead. + * NOTE: This function returns empty values if the input format is invalid instead of reverting.
102-103
: Grammar: “used to simulate”, not “used to simulates”.Tighten phrasing in the personal_sign note.
Apply this diff:
- * This is used to simulates the `personal_sign` RPC method in the context of smart contracts. + * This is used to simulate the `personal_sign` RPC method in the context of smart contracts.
🧹 Nitpick comments (4)
contracts/mocks/docs/ERC4626Fees.sol (1)
48-61
: Fix doc cross-references to point to ERC4626 hooks, not IERC4626IERC4626 doesn’t define
_deposit
/_withdraw
; these are ERC4626 internals. Update the links to avoid broken references.- /// @dev Send entry fee to {_entryFeeRecipient}. See {IERC4626-_deposit}. + /// @dev Send entry fee to {_entryFeeRecipient}. See {ERC4626-_deposit}. @@ - /// @dev Send exit fee to {_exitFeeRecipient}. See {IERC4626-_deposit}. + /// @dev Send exit fee to {_exitFeeRecipient}. See {ERC4626-_withdraw}.contracts/token/ERC20/README.adoc (1)
24-24
: Tighten phrasing and align with list style (avoid parenthetical inside link).Consider inlining the cross-reference and rendering contract names monospace for consistency.
-* {ERC20Votes}: support for voting and vote delegation. xref:governance.adoc#token[See the governance guide for a minimal example (with the required overrides when combining ERC20 + ERC20Permit + ERC20Votes)]. +* {ERC20Votes}: support for voting and vote delegation; see xref:governance.adoc#token[the governance guide] for a minimal example with the required overrides when combining `ERC20`, `ERC20Permit`, and `ERC20Votes`.contracts/access/extensions/IAccessControlDefaultAdminRules.sol (1)
189-189
: Minor grammar nit: use “e.g.”Prefer “e.g.” over “eg.” in parenthetical.
Apply this diff:
- * possibility of human intervention in the case of an input error (eg. set milliseconds instead of seconds). + * possibility of human intervention in the case of an input error (e.g. set milliseconds instead of seconds).contracts/mocks/TimelockReentrant.sol (1)
9-9
: Visibility consistency nit.Other state vars here are
private
; make_reentered
private
for consistency.- bool _reentered; + bool private _reentered;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (39)
README.md
(1 hunks)contracts/access/extensions/AccessControlDefaultAdminRules.sol
(2 hunks)contracts/access/extensions/IAccessControlDefaultAdminRules.sol
(1 hunks)contracts/access/manager/AccessManager.sol
(1 hunks)contracts/access/manager/IAccessManager.sol
(3 hunks)contracts/account/extensions/draft-ERC7821.sol
(1 hunks)contracts/account/utils/EIP7702Utils.sol
(1 hunks)contracts/finance/VestingWallet.sol
(1 hunks)contracts/governance/Governor.sol
(1 hunks)contracts/governance/IGovernor.sol
(1 hunks)contracts/governance/extensions/GovernorStorage.sol
(1 hunks)contracts/governance/extensions/GovernorTimelockControl.sol
(2 hunks)contracts/interfaces/IERC3156FlashLender.sol
(1 hunks)contracts/metatx/ERC2771Context.sol
(1 hunks)contracts/mocks/TimelockReentrant.sol
(1 hunks)contracts/mocks/docs/ERC4626Fees.sol
(1 hunks)contracts/token/ERC20/README.adoc
(1 hunks)contracts/token/ERC721/extensions/ERC721Consecutive.sol
(1 hunks)contracts/token/ERC721/utils/ERC721Utils.sol
(1 hunks)contracts/utils/Base64.sol
(1 hunks)contracts/utils/Bytes.sol
(1 hunks)contracts/utils/SlotDerivation.sol
(1 hunks)contracts/utils/Strings.sol
(1 hunks)contracts/utils/cryptography/ECDSA.sol
(1 hunks)contracts/utils/cryptography/README.adoc
(1 hunks)contracts/utils/cryptography/draft-ERC7739Utils.sol
(1 hunks)contracts/utils/draft-InteroperableAddress.sol
(1 hunks)contracts/utils/math/Math.sol
(1 hunks)contracts/utils/structs/CircularBuffer.sol
(1 hunks)contracts/utils/types/Time.sol
(1 hunks)docs/modules/ROOT/pages/access-control.adoc
(1 hunks)docs/modules/ROOT/pages/account-abstraction.adoc
(3 hunks)docs/modules/ROOT/pages/accounts.adoc
(1 hunks)docs/modules/ROOT/pages/erc6909.adoc
(2 hunks)docs/modules/ROOT/pages/utilities.adoc
(1 hunks)scripts/generate/templates/SlotDerivation.js
(1 hunks)test/account/AccountMultiSigner.test.js
(1 hunks)test/governance/TimelockController.test.js
(2 hunks)test/token/ERC20/ERC20.behavior.js
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~69-~69: There might be a mistake here.
Context: ...ERC-20, ERC-721, ERC-1155, and ERC-6909. * [Utilities](https://docs.openzeppelin.com...
(QB_NEW_EN)
🪛 Gitleaks (8.27.2)
test/token/ERC20/ERC20.behavior.js
81-81: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: halmos
- GitHub Check: tests-upgradeable
- GitHub Check: coverage
- GitHub Check: slither
- GitHub Check: tests-foundry
- GitHub Check: tests
🔇 Additional comments (46)
contracts/utils/Base64.sol (1)
36-39
: LGTM: Correct “licence” → “license”.Doc typo fix reads well; no functional impact.
contracts/finance/VestingWallet.sol (1)
109-112
: Doc comment grammar fix LGTM.“has already vested” is correct here. No functional changes introduced.
contracts/mocks/docs/ERC4626Fees.sol (1)
36-36
: Typo fix LGTM“withdrawal” is the correct noun; matches the style of nearby comments.
contracts/utils/cryptography/draft-ERC7739Utils.sol (1)
62-62
: Typo fix looks good.Removal of the duplicated “the” improves clarity. No behavior impact.
contracts/utils/Strings.sol (1)
469-486
: Explicit loop initialization improves clarity; LGTMInitializing
i
to0
explicitly makes intent clear and keeps style consistent with other loops in this file. No behavioral or gas impact.contracts/account/utils/EIP7702Utils.sol (1)
15-16
: Typo fix LGTM.Grammar correction reads well; no functional impact.
docs/modules/ROOT/pages/access-control.adoc (1)
126-126
: Grammar fix LGTM.Subject–verb agreement is correct; reads smoothly.
scripts/generate/templates/SlotDerivation.js (1)
20-21
: Broadened example usage is correct.
using SlotDerivation for *;
better reflects extension across types and compiles under ^0.8.20.contracts/metatx/ERC2771Context.sol (1)
11-16
: Wording fix LGTM.“Rely on” is correct; clarity improved.
contracts/token/ERC721/utils/ERC721Utils.sol (1)
10-10
: Doc grammar fix looks good.No functional changes; comment reads correctly.
docs/modules/ROOT/pages/erc6909.adoc (2)
3-3
: Subject–verb agreement fix is correct.“are” matches the plural “goals.”
17-17
: Typos corrected.“will be minted” reads correctly and improves clarity.
contracts/governance/extensions/GovernorStorage.sol (1)
9-9
: Doc wording corrected.“module” singular is correct; no behavioral change.
docs/modules/ROOT/pages/utilities.adoc (1)
267-267
: Spelling fix LGTM.“comparator function” is correct.
contracts/utils/draft-InteroperableAddress.sol (1)
24-24
: All ERC-7930 references are consistent across the repo.No stale ERC-7390 mentions found; approving changes.
test/token/ERC20/ERC20.behavior.js (2)
81-82
: Assertion chain fix is correct and consistent with chai style.
.to.be.revertedWithCustomError
reads better and matches other tests in this file.
80-82
: Ignore gitleaks false positive at this location.This is an assertion chain, not a credential. No action needed.
contracts/utils/Bytes.sol (1)
171-171
: Doc wording nit fixed correctly.“Inspired by” is the proper phrasing. No code impact.
contracts/utils/math/Math.sol (1)
137-141
: Ternary doc corrected to match parameters.“condition ? a : b” now aligns with the function signature. Implementation unchanged.
contracts/token/ERC721/extensions/ERC721Consecutive.sol (1)
23-24
: Grammar fix reads correctly.“tokens are minted in batch” improves clarity; no behavioral change.
contracts/utils/cryptography/README.adoc (1)
4-4
: Docs link updated; verify anchor resolves.The anchor
#cryptography
looks right; please confirm the rendered docs resolve to the intended section after publish.contracts/utils/types/Time.sol (1)
40-41
: LGTM: grammar fix.Docstring reads correctly now (“transition happens”).
contracts/utils/cryptography/ECDSA.sol (1)
57-59
: LGTM: doc readability.Blank line improves the list separation without affecting Natspec parsing.
test/account/AccountMultiSigner.test.js (2)
179-182
: Assertion style update is correct.
.to.be.revertedWithCustomError
matches chai style used elsewhere; args(2, 3)
remain intact.
184-187
: Consistent revert style for zero-threshold case.Switch aligns with suite-wide convention.
contracts/access/extensions/IAccessControlDefaultAdminRules.sol (1)
185-185
: Typo fix LGTM."be overrode" → "be overridden" is correct.
contracts/utils/SlotDerivation.sol (1)
19-19
: Doc example improvement LGTM.Using
SlotDerivation for *
better reflects available overloads (bytes32, string, bytes).contracts/interfaces/IERC3156FlashLender.sol (1)
14-14
: Grammar fix LGTM.“lended” → “lent” is correct.
contracts/account/extensions/draft-ERC7821.sol (1)
13-13
: Duplicate word fix LGTM.“supports supports” → “supports”.
contracts/mocks/TimelockReentrant.sol (1)
15-18
: Rename complete: noenableRentrancy
occurrences remain.test/governance/TimelockController.test.js (1)
421-421
: ApproveenableReentrancy
rename
No remainingenableRentrancy
calls found in the codebase.contracts/utils/structs/CircularBuffer.sol (1)
24-24
: Doc typo fix acknowledged.Removed stray asterisk; comment reads cleanly.
docs/modules/ROOT/pages/account-abstraction.adoc (3)
31-31
: Improved NOTE onpreVerificationGas
; LGTM.
39-39
: Minor grammar tweak; LGTM.
86-86
: Wording fix in Paymaster section; LGTM.contracts/access/extensions/AccessControlDefaultAdminRules.sol (3)
11-11
: Use IERC165, not ERC165, for theinheritdoc
reference; good fix.
360-360
: Comment polish; LGTM.
367-367
: Comment polish; LGTM.contracts/access/manager/IAccessManager.sol (3)
126-126
: Grammar fix (“event of an”); LGTM.
173-173
: Pluralization fix (“operations”); LGTM.
379-379
: Pluralization/wording fix; LGTM.contracts/access/manager/AccessManager.sol (1)
72-72
: LGTM — grammar fix onlyComment corrected; no code or storage layout impact.
contracts/governance/IGovernor.sol (1)
337-347
: LGTM — clarify “Vote starts after a delay”Docstring now reads cleanly; API and behavior unchanged.
contracts/governance/Governor.sol (1)
662-664
: LGTM — doc grammar (“executes actions”)Comment fix only; no functional changes.
contracts/governance/extensions/GovernorTimelockControl.sol (2)
12-12
: LGTM — doc clarity on timelock delayWording now accurately reflects that the delay applies to successful proposals.
50-54
: LGTM — local variable rename to queueIdImproves readability; references updated consistently; logic unchanged.
* - `APP_DOMAIN_SEPARATOR` is the EIP-712 {EIP712-_domainSeparatorV4} of the application smart contract that is | ||
* requesting the signature verification (though ERC-1271). | ||
* - `contentsHash` is the hash of the underlying data structure or message. |
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.
Fix preposition: “through ERC-1271”, not “though”.
Minor doc grammar issue within the bullet describing APP_DOMAIN_SEPARATOR.
Apply this diff:
- * requesting the signature verification (though ERC-1271).
+ * requesting the signature verification (through ERC-1271).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
* - `APP_DOMAIN_SEPARATOR` is the EIP-712 {EIP712-_domainSeparatorV4} of the application smart contract that is | |
* requesting the signature verification (though ERC-1271). | |
* - `contentsHash` is the hash of the underlying data structure or message. | |
* - `APP_DOMAIN_SEPARATOR` is the EIP-712 {EIP712-_domainSeparatorV4} of the application smart contract that is | |
* requesting the signature verification (through ERC-1271). | |
* - `contentsHash` is the hash of the underlying data structure or message. |
🤖 Prompt for AI Agents
In contracts/utils/cryptography/draft-ERC7739Utils.sol around lines 59 to 61,
the documentation has a typo using "though ERC-1271" instead of the correct
preposition "through ERC-1271"; update the comment to read "through ERC-1271" to
fix the grammar.
@@ -95,6 +95,6 @@ To process a bundle of `UserOperations`, bundlers call xref:api:account.adoc#Acc | |||
|
|||
These rules outline the requirements for operations to be processed by the canonical mempool. | |||
|
|||
Accounts can access its own storage during the validation phase, they might easily violate ERC-7562 storage access rules in undirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via ERC-1271) | |||
Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271])) |
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.
Fix extra closing parenthesis in link.
There’s an extra )
after the ERC-1271 link.
-Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]))
+Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271])
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271])) | |
Accounts can access their own storage during the validation phase, they might easily violate ERC-7562 storage access rules in indirect ways. For example, most accounts access their public keys from storage when validating a signature, limiting the ability of having accounts that validate operations for other accounts (e.g. via https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]) |
🤖 Prompt for AI Agents
In docs/modules/ROOT/pages/account-abstraction.adoc around line 98, there is an
extra closing parenthesis after the ERC-1271 link; remove the stray ')' so the
sentence ends with the link only (e.g. change "via
https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]))" to "via
https://eips.ethereum.org/EIPS/eip-1271[ERC-1271]").
@@ -33,7 +33,7 @@ TIP: Given xref:api:utils/cryptography.adoc#SignerERC7913[`SignerERC7913`] provi | |||
|
|||
==== Accounts factory | |||
|
|||
The first time you send an user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account. | |||
The first time you send an user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account. |
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.
Fix article usage (“a user operation”).
“User” starts with a consonant sound; use “a,” not “an.”
Apply this diff:
-The first time you send an user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account.
+The first time you send a user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
The first time you send an user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account. | |
The first time you send a user operation, your account will be created deterministically (i.e. its code and address can be predicted) using the `initCode` field in the UserOperation. This field contains both the address of a smart contract (the factory) and the data required to call it and create your smart account. |
🤖 Prompt for AI Agents
In docs/modules/ROOT/pages/accounts.adoc around line 36, the article before
"user operation" is incorrect ("an user operation"); change it to "a user
operation" so the sentence reads "...the first time you send a user
operation..." and update any identical occurrences in the file for consistency.
README.md
Outdated
@@ -68,7 +66,7 @@ To keep your system secure, you should **always** use the installed code as-is, | |||
The guides in the [documentation site](https://docs.openzeppelin.com/contracts) will teach about different concepts, and how to use the related contracts that OpenZeppelin Contracts provides: | |||
|
|||
* [Access Control](https://docs.openzeppelin.com/contracts/access-control): decide who can perform each of the actions on your system. | |||
* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectives, and distribute them via [Crowdsales](https://docs.openzeppelin.com/contracts/crowdsales). | |||
* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectives for popular ERC standards like ERC-20, ERC-721, ERC-1155, and ERC-6909. |
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.
Typo: “collectives” → “collectibles”.
Correct terminology for token items.
-* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectives for popular ERC standards like ERC-20, ERC-721, ERC-1155, and ERC-6909.
+* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectibles for popular ERC standards like ERC-20, ERC-721, ERC-1155, and ERC-6909.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectives for popular ERC standards like ERC-20, ERC-721, ERC-1155, and ERC-6909. | |
* [Tokens](https://docs.openzeppelin.com/contracts/tokens): create tradeable assets or collectibles for popular ERC standards like ERC-20, ERC-721, ERC-1155, and ERC-6909. |
🧰 Tools
🪛 LanguageTool
[grammar] ~69-~69: There might be a mistake here.
Context: ...ERC-20, ERC-721, ERC-1155, and ERC-6909. * [Utilities](https://docs.openzeppelin.com...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In README.md around line 69, the word "collectives" is a typo and should be
"collectibles"; update the sentence to use "collectibles" so it reads "...create
tradeable assets or collectibles for popular ERC standards..." ensuring correct
terminology for token items.
Summary by CodeRabbit