docs(ERC2771Forwarder): fix validate signer reference and clarify implicit nonce #5938
Conversation
|
WalkthroughUpdated comments in contracts/metatx/ERC2771Forwarder.sol. Clarified that the nonce is implicit, taken from Nonces for the from address and included in the signed data, rather than being a separate explicit field. Adjusted validation commentary to state that the signature is on behalf of request.from instead of request.signer. No executable logic, types, or public API were modified. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
contracts/metatx/ERC2771Forwarder.sol (1)
22-22: LGTM: implicit nonce doc fix is accurate.Matches implementation: nonce is injected from
Noncesduring hashing (nonces(request.from)) and consumed via_useNonce(signer)before the call.Optional: Append “consumed via {_useNonce} prior to execution” to make nonce consumption explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/metatx/ERC2771Forwarder.sol(2 hunks)
⏰ 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: slither
- GitHub Check: coverage
- GitHub Check: tests
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: halmos
| * the given `request.signature` on behalf of `request.from`. | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
LGTM: validation wording correctly ties signature to request.from.
Consistent with _validate and _recoverForwardRequestSigner.
Run to ensure no lingering references to request.signer in docs/comments:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\brequest\.signer\b|request signer' --type Solidity --type md --iglob '!**/node_modules/**'Length of output: 32
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Re-run search without relying on rg's Solidity filetype
rg -nP -C2 '\brequest\.signer\b|request signer' --glob '**/*.sol' --glob '**/*.md' -g '!**/node_modules/**' || trueLength of output: 2245
LGTM: validation wording correctly ties signature to request.from. Fix remaining 'request signer' mentions in docs/comments.
Consistent with _validate and _recoverForwardRequestSigner.
- contracts/metatx/ERC2771Forwarder.sol:18 — "*
from: An address to operate on behalf of. It is required to be equal to the request signer." - CHANGELOG.md:376–377, 566–567 — references to "request signer address"
Replace "request signer" with request.from or reword to "signature must be from request.from".
🤖 Prompt for AI Agents
In contracts/metatx/ERC2771Forwarder.sol (around lines 18 and 198–199) and
CHANGELOG.md (lines ~376–377 and ~566–567), there are outdated references to
"request signer" — update the text to reference `request.from` or rephrase to
"signature must be from request.from" so the docs/comments consistently tie the
signature to request.from; edit the specific lines to replace "request signer"
with the literal `request.from` or the suggested rewording, preserving
surrounding punctuation and formatting.
Replace “request.signer” with “request.from” in validate NatSpec to match the actual struct field and validation logic.
Clarify that the request nonce is implicit (sourced from Nonces for from and included in the typed data) rather than being a field in ForwardRequestData.