Add Paymaster, PaymasterERC20, PaymasterERC20Guarantor, PaymasterERC721Owner and PaymasterSigner#6370
Draft
ernestognw wants to merge 5 commits intoOpenZeppelin:masterfrom
Draft
Add Paymaster, PaymasterERC20, PaymasterERC20Guarantor, PaymasterERC721Owner and PaymasterSigner#6370ernestognw wants to merge 5 commits intoOpenZeppelin:masterfrom
ernestognw wants to merge 5 commits intoOpenZeppelin:masterfrom
Conversation
…21Owner and PaymasterSigner
🦋 Changeset detectedLatest commit: ded2d62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
WalkthroughThis PR introduces a comprehensive ERC-4337 Paymaster implementation framework. It adds a base abstract Paymaster contract providing entry point enforcement, fund management, and extensible hooks for validation and post-operation logic. Four specialized paymaster extensions are introduced: PaymasterERC20 for ERC-20 token-based gas sponsorship, PaymasterERC20Guarantor for third-party guarantor support, PaymasterERC721Owner for NFT ownership-based sponsorship, and PaymasterSigner for signature-based sponsorship. The PR includes corresponding mock contracts for testing scenarios, comprehensive test suites validating paymaster behavior and user operation flows, and extensive documentation describing ERC-4337 paymasters with code examples and implementation guidance. Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
contracts/mocks/docs/account/paymaster/PaymasterECDSASigner.sol (1)
11-11: Consider separating signer and owner roles — or add an inline comment clarifying the coupling.
Ownable(signerAddr)makes the ECDSA signing key the Ownable owner, so the same address that authorizes user operations also controls fund withdrawals. For a docs/mock contract this is a deliberate simplification, but it could mislead readers into treating the signer and the withdrawal-authorized owner as inherently the same role in production.If this is intended as an illustrative shortcut, a brief inline comment would improve clarity. Alternatively, the constructor could accept a distinct
initialOwnerparameter to better reflect the general case:♻️ Proposed refactor to separate roles
- constructor(address signerAddr) EIP712("MyPaymasterECDSASigner", "1") Ownable(signerAddr) SignerECDSA(signerAddr) {} + constructor(address initialOwner, address signerAddr) EIP712("MyPaymasterECDSASigner", "1") Ownable(initialOwner) SignerECDSA(signerAddr) {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/mocks/docs/account/paymaster/PaymasterECDSASigner.sol` at line 11, The constructor currently couples the ECDSA signer and contract owner by calling Ownable(signerAddr) in PaymasterECDSASigner, which can mislead readers; either add a brief inline comment next to the constructor/Ownable(signerAddr) call stating this is an intentional mock simplification and that signer and withdrawal-owner are intentionally the same here, or change the constructor signature of PaymasterECDSASigner to accept a separate initialOwner parameter and initialize Ownable(initialOwner) while keeping SignerECDSA(signerAddr) and EIP712(...) unchanged so the signing key and owner roles are clearly separated.test/account/paymaster/Paymaster.behavior.js (1)
191-197: Prefervalueover hardcoded42nin expected stake assertions.Using the shared constant here keeps the test resilient if the fixture amount changes.
♻️ Suggested tweak
await expect(predeploy.entrypoint.v09.getDepositInfo(this.paymaster)).to.eventually.deep.equal([ 0n, true, - 42n, + value, delay, 0n, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/account/paymaster/Paymaster.behavior.js` around lines 191 - 197, The assertion uses a hardcoded stake amount 42n; replace that literal with the shared fixture constant (value) so the expectation stays in sync with the fixture—update the expected array in the getDepositInfo assertion (predeploy.entrypoint.v09.getDepositInfo(this.paymaster)) to use value instead of 42n.contracts/account/paymaster/PaymasterERC20.sol (1)
162-164: Add a guard foractualAmount_ > prefundAmountbefore subtraction.Line 163 can panic on underflow if estimates drift. Returning
(false, actualAmount_)preserves the intendedPaymasterERC20FailedRefundpath in_postOp.Suggested hardening
uint256 actualAmount_ = _erc20Cost(actualGasCost, actualUserOpFeePerGas, tokenPrice); + if (actualAmount_ > prefundAmount) { + return (false, actualAmount_); + } return (token.trySafeTransfer(prefunder, prefundAmount - actualAmount_), actualAmount_);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/account/paymaster/PaymasterERC20.sol` around lines 162 - 164, The subtraction prefundAmount - actualAmount_ can underflow if actualAmount_ > prefundAmount; in the function that computes actualAmount_ via _erc20Cost and attempts the refund with token.trySafeTransfer(prefunder, prefundAmount - actualAmount_), add a guard: if actualAmount_ > prefundAmount then skip the transfer and return (false, actualAmount_) so the PaymasterERC20FailedRefund path in _postOp is preserved; otherwise perform the trySafeTransfer as before. Reference _erc20Cost, actualAmount_, prefundAmount, token.trySafeTransfer and the PaymasterERC20FailedRefund handling in _postOp when adding the check and early return.contracts/account/paymaster/PaymasterERC20Guarantor.sol (1)
67-106: Decode sender from the tail ofprefundContextand pass upstream context unchanged.Line 67 appends
userOp.sender, but Line 88 reads from the beginning and Line 105 forwards the augmented context directly tosuper._refund. This makes extension layering brittle if parent/child contexts are added later.Suggested refactor
- address userOpSender = address(bytes20(prefundContext[0x00:0x14])); + if (prefundContext.length < 0x14) { + return + super._refund( + token, + tokenPrice, + actualGasCost, + actualUserOpFeePerGas, + prefunder, + prefundAmount, + prefundContext + ); + } + + uint256 senderOffset = prefundContext.length - 0x14; + address userOpSender = address(bytes20(prefundContext[senderOffset:])); + bytes calldata upstreamPrefundContext = prefundContext[:senderOffset]; @@ - prefundContext + upstreamPrefundContext );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/account/paymaster/PaymasterERC20Guarantor.sol` around lines 67 - 106, The code currently decodes userOp.sender from the start of prefundContext and forwards the augmented prefundContext to super._refund, making layering brittle; change _refund to extract the sender from the tail (last 20 bytes) and pass the original upstream context (prefundContext without the trailing 20-byte sender) to super._refund. Concretely, in function _refund compute userOpSender = address(bytes20(prefundContext[prefundContext.length - 20 : prefundContext.length])), set upstreamContext = prefundContext[0 : prefundContext.length - 20] (and revert or handle if prefundContext.length < 20), use userOpSender for the transfer logic, and forward upstreamContext (not the augmented prefundContext) when calling super._refund so parent contracts receive the unmodified context.test/account/paymaster/PaymasterERC20.test.js (1)
245-288: Add no-ether-change checks to the revert scenarios.For these
FailedOptests, please also assert that ether balances do not change for the relevant actors. This makes the revert-path expectations complete and prevents silent side-effect regressions.Based on learnings: In tests that exercise revert semantics, use negative balance assertions to verify no ether balance changes occurred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/account/paymaster/PaymasterERC20.test.js` around lines 245 - 288, Add explicit no-ether-change assertions to each revert test ('reverts with an invalid token', 'reverts with insufficient balance', 'reverts with insufficient approval'): record or assert Ether balances for the relevant actors (at minimum this.account, this.paymaster and this.receiver) before calling predeploy.entrypoint.v09.handleOps(signedUserOp.packed, this.receiver) and assert they do not change after the call (e.g. assert zero delta with Chai's changeEtherBalances or by comparing balances before/after). Update the three tests so the revert expectation is accompanied by these no-balance-change checks around predeploy.entrypoint.v09.handleOps to prevent silent ether side-effects.test/account/paymaster/PaymasterERC20Guarantor.test.js (1)
344-390: Add explicit no-ether-movement assertions for the revert-path cases.These three tests validate the revert reason, but they don’t verify that balances are unchanged after revert. Please add a no-balance-change assertion for the EntryPoint/beneficiary pair to guard against unintended side effects.
Based on learnings: In tests that exercise revert semantics, use negative balance assertions to verify no ether balance changes occurred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/account/paymaster/PaymasterERC20Guarantor.test.js` around lines 344 - 390, After each revert assertion, capture and compare balances before and after the call to ensure no ether moved: read balances with ethers.provider.getBalance for predeploy.entrypoint.v09.address and this.receiver (e.g., const beforeEP = await ethers.provider.getBalance(predeploy.entrypoint.v09.address); const beforeR = await ethers.provider.getBalance(this.receiver);), perform the existing expect(...).to.be.revertedWithCustomError(...) call, then re-read balances and assert they are unchanged with expect(afterEP).to.equal(beforeEP) and expect(afterR).to.equal(beforeR). Do this for the three tests that use predeploy.entrypoint.v09.handleOps to validate the EntryPoint/beneficiary pair has zero net ether change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/dark-papers-call.md:
- Line 5: The changeset description incorrectly references PaymasterCore; update
the sentence mentioning PaymasterCore to reference Paymaster instead so it reads
that PaymasterERC721Owner is an extension of Paymaster that approves sponsoring
based on ERC-721 ownership (update the text for the symbol PaymasterERC721Owner
and replace PaymasterCore with Paymaster).
In @.changeset/polite-geckos-peel.md:
- Line 5: The release note for PaymasterERC20 incorrectly names the base
contract as PaymasterCore; update the sentence that currently reads
"`PaymasterERC20`: Extension of `PaymasterCore` that sponsors user operations
against payment in ERC-20 tokens." to reference the actual base type `Paymaster`
instead (i.e., replace `PaymasterCore` with `Paymaster`) so it accurately
describes PaymasterERC20 as an extension of Paymaster.
In @.changeset/shy-poets-look.md:
- Line 5: Update the changeset text to reference the correct base contract name:
replace "PaymasterCore" with "Paymaster" in the description for PaymasterSigner
so it reads that PaymasterSigner is an extension of Paymaster that approves
sponsoring of user operations via cryptographic signatures; ensure the symbol
names PaymasterSigner and Paymaster are used exactly as in the diff.
In `@contracts/account/paymaster/Paymaster.sol`:
- Around line 132-134: The NatSpec example incorrectly references
_authorizeUpgrade() instead of the withdraw hook; update the comment so the
example shows calling _authorizeWithdraw() (e.g., replace `_authorizeUpgrade()`
with `_authorizeWithdraw()` in the documentation block) to ensure implementers
use the correct hook when overriding the Paymaster's authorization logic.
In `@contracts/account/paymaster/PaymasterERC20Guarantor.sol`:
- Around line 61-63: The post-op surcharge is applied to the wrong branch: when
building the call that currently uses isGuaranteed ? guarantor : prefunder_ and
maxCost + (isGuaranteed ? 0 : _guaranteedPostOpCost()), flip the conditional for
the surcharge so the guaranteed post-op cost is added when isGuaranteed is true.
Update the expression that uses maxCost and _guaranteedPostOpCost() (the same
call constructing the payment/cost) to add _guaranteedPostOpCost() when
isGuaranteed is true and 0 otherwise, leaving the address selection
(isGuaranteed ? guarantor : prefunder_) unchanged.
In `@contracts/account/paymaster/PaymasterSigner.sol`:
- Line 3: The PaymasterSigner.sol file uses a different Solidity pragma
(^0.8.24) than the rest of the paymaster contracts (^0.8.20); update the pragma
in PaymasterSigner.sol to ^0.8.20 to match Paymaster.sol, PaymasterERC20.sol,
PaymasterERC721Owner.sol and PaymasterERC20Guarantor.sol so the compiler floor
is consistent across the paymaster family.
In `@contracts/account/README.adoc`:
- Line 16: Update the README entry for PaymasterERC721Owner to avoid implying a
payment flow: replace wording that says it "allows users to pay for user
operations based on ERC-721 ownership" with a statement that ownership of an
ERC-721 token grants eligibility for sponsorship (the paymaster
covers/gas-sponsors user operations for NFT holders at no cost), and contrast
this with PaymasterERC20 which implements a payment model; ensure the symbol
PaymasterERC721Owner is used and mention PaymasterERC20 for clarity.
In `@docs/modules/ROOT/pages/paymasters.adoc`:
- Around line 372-379: The current guard checks guarantor == address(0) before
guarantor is assigned (it’s the named zero-initialized return), so change the
logic to first validate paymasterData length, then parse and assign guarantor
using guarantor = address(bytes20(paymasterData[:20])), and only after that
check if guarantor == address(0) (or simply remove the redundant zero check if
you only meant to early-return on insufficient length); adjust related variables
like uint16 guarantorSigLength = uint16(bytes2(paymasterData[20:22])) to remain
after the length check/assignment.
---
Nitpick comments:
In `@contracts/account/paymaster/PaymasterERC20.sol`:
- Around line 162-164: The subtraction prefundAmount - actualAmount_ can
underflow if actualAmount_ > prefundAmount; in the function that computes
actualAmount_ via _erc20Cost and attempts the refund with
token.trySafeTransfer(prefunder, prefundAmount - actualAmount_), add a guard: if
actualAmount_ > prefundAmount then skip the transfer and return (false,
actualAmount_) so the PaymasterERC20FailedRefund path in _postOp is preserved;
otherwise perform the trySafeTransfer as before. Reference _erc20Cost,
actualAmount_, prefundAmount, token.trySafeTransfer and the
PaymasterERC20FailedRefund handling in _postOp when adding the check and early
return.
In `@contracts/account/paymaster/PaymasterERC20Guarantor.sol`:
- Around line 67-106: The code currently decodes userOp.sender from the start of
prefundContext and forwards the augmented prefundContext to super._refund,
making layering brittle; change _refund to extract the sender from the tail
(last 20 bytes) and pass the original upstream context (prefundContext without
the trailing 20-byte sender) to super._refund. Concretely, in function _refund
compute userOpSender = address(bytes20(prefundContext[prefundContext.length - 20
: prefundContext.length])), set upstreamContext = prefundContext[0 :
prefundContext.length - 20] (and revert or handle if prefundContext.length <
20), use userOpSender for the transfer logic, and forward upstreamContext (not
the augmented prefundContext) when calling super._refund so parent contracts
receive the unmodified context.
In `@contracts/mocks/docs/account/paymaster/PaymasterECDSASigner.sol`:
- Line 11: The constructor currently couples the ECDSA signer and contract owner
by calling Ownable(signerAddr) in PaymasterECDSASigner, which can mislead
readers; either add a brief inline comment next to the
constructor/Ownable(signerAddr) call stating this is an intentional mock
simplification and that signer and withdrawal-owner are intentionally the same
here, or change the constructor signature of PaymasterECDSASigner to accept a
separate initialOwner parameter and initialize Ownable(initialOwner) while
keeping SignerECDSA(signerAddr) and EIP712(...) unchanged so the signing key and
owner roles are clearly separated.
In `@test/account/paymaster/Paymaster.behavior.js`:
- Around line 191-197: The assertion uses a hardcoded stake amount 42n; replace
that literal with the shared fixture constant (value) so the expectation stays
in sync with the fixture—update the expected array in the getDepositInfo
assertion (predeploy.entrypoint.v09.getDepositInfo(this.paymaster)) to use value
instead of 42n.
In `@test/account/paymaster/PaymasterERC20.test.js`:
- Around line 245-288: Add explicit no-ether-change assertions to each revert
test ('reverts with an invalid token', 'reverts with insufficient balance',
'reverts with insufficient approval'): record or assert Ether balances for the
relevant actors (at minimum this.account, this.paymaster and this.receiver)
before calling predeploy.entrypoint.v09.handleOps(signedUserOp.packed,
this.receiver) and assert they do not change after the call (e.g. assert zero
delta with Chai's changeEtherBalances or by comparing balances before/after).
Update the three tests so the revert expectation is accompanied by these
no-balance-change checks around predeploy.entrypoint.v09.handleOps to prevent
silent ether side-effects.
In `@test/account/paymaster/PaymasterERC20Guarantor.test.js`:
- Around line 344-390: After each revert assertion, capture and compare balances
before and after the call to ensure no ether moved: read balances with
ethers.provider.getBalance for predeploy.entrypoint.v09.address and
this.receiver (e.g., const beforeEP = await
ethers.provider.getBalance(predeploy.entrypoint.v09.address); const beforeR =
await ethers.provider.getBalance(this.receiver);), perform the existing
expect(...).to.be.revertedWithCustomError(...) call, then re-read balances and
assert they are unchanged with expect(afterEP).to.equal(beforeEP) and
expect(afterR).to.equal(beforeR). Do this for the three tests that use
predeploy.entrypoint.v09.handleOps to validate the EntryPoint/beneficiary pair
has zero net ether change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.changeset/dark-papers-call.md.changeset/polite-geckos-peel.md.changeset/shy-poets-look.md.changeset/whole-items-rule.mdcontracts/account/README.adoccontracts/account/paymaster/Paymaster.solcontracts/account/paymaster/PaymasterERC20.solcontracts/account/paymaster/PaymasterERC20Guarantor.solcontracts/account/paymaster/PaymasterERC721Owner.solcontracts/account/paymaster/PaymasterSigner.solcontracts/mocks/account/paymaster/PaymasterERC20Mock.solcontracts/mocks/account/paymaster/PaymasterERC721OwnerMock.solcontracts/mocks/account/paymaster/PaymasterSignerMock.solcontracts/mocks/docs/account/paymaster/PaymasterECDSASigner.solcontracts/mocks/token/ERC20BlocklistMock.soldocs/modules/ROOT/nav.adocdocs/modules/ROOT/pages/account-abstraction.adocdocs/modules/ROOT/pages/eoa-delegation.adocdocs/modules/ROOT/pages/paymasters.adoctest/account/paymaster/Paymaster.behavior.jstest/account/paymaster/PaymasterERC20.test.jstest/account/paymaster/PaymasterERC20Guarantor.test.jstest/account/paymaster/PaymasterERC721Owner.test.jstest/account/paymaster/PaymasterSigner.test.js
Comment on lines
+61
to
+63
| isGuaranteed ? guarantor : prefunder_, | ||
| maxCost + (isGuaranteed ? 0 : _guaranteedPostOpCost()) | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "PaymasterERC20Guarantor.sol"Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 136
🏁 Script executed:
cat -n contracts/account/paymaster/PaymasterERC20Guarantor.sol | head -100Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 5126
🏁 Script executed:
cat -n contracts/account/paymaster/PaymasterERC20Guarantor.sol | sed -n '50,80p'Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 1625
🏁 Script executed:
# Search for _guaranteedPostOpCost definition
rg "_guaranteedPostOpCost" -B 2 -A 8 contracts/account/paymaster/PaymasterERC20Guarantor.solRepository: OpenZeppelin/openzeppelin-contracts
Length of output: 713
🏁 Script executed:
# Look for comments or documentation about guaranteed/non-guaranteed paths
rg -i "guaranteed|prefund|surcharge" -B 2 -A 2 contracts/account/paymaster/PaymasterERC20Guarantor.sol | head -60Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 2509
Guaranteed post-op surcharge is applied to the wrong branch.
At line 62, _guaranteedPostOpCost() is added when isGuaranteed is false. This is inverted: the surcharge should apply to guaranteed operations (when a guarantor takes on repayment risk), not to non-guaranteed operations.
Suggested fix
- maxCost + (isGuaranteed ? 0 : _guaranteedPostOpCost())
+ maxCost + (isGuaranteed ? _guaranteedPostOpCost() : 0)📝 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.
Suggested change
| isGuaranteed ? guarantor : prefunder_, | |
| maxCost + (isGuaranteed ? 0 : _guaranteedPostOpCost()) | |
| ); | |
| isGuarantee ? guarantor : prefunder_, | |
| maxCost + (isGuaranteed ? _guaranteedPostOpCost() : 0) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/account/paymaster/PaymasterERC20Guarantor.sol` around lines 61 -
63, The post-op surcharge is applied to the wrong branch: when building the call
that currently uses isGuaranteed ? guarantor : prefunder_ and maxCost +
(isGuaranteed ? 0 : _guaranteedPostOpCost()), flip the conditional for the
surcharge so the guaranteed post-op cost is added when isGuaranteed is true.
Update the expression that uses maxCost and _guaranteedPostOpCost() (the same
call constructing the payment/cost) to add _guaranteedPostOpCost() when
isGuaranteed is true and 0 otherwise, leaving the address selection
(isGuaranteed ? guarantor : prefunder_) unchanged.
Comment on lines
+372
to
+379
| // Check guarantor data (should be at least 22 bytes: 20 for address + 2 for sig length) | ||
| // If no guarantor specified, return early | ||
| if (paymasterData.length < 22 || guarantor == address(0)) { | ||
| return address(0); | ||
| } | ||
|
|
||
| guarantor = address(bytes20(paymasterData[:20])); | ||
| uint16 guarantorSigLength = uint16(bytes2(paymasterData[20:22])); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n docs/modules/ROOT/pages/paymasters.adoc | sed -n '365,385p'Repository: OpenZeppelin/openzeppelin-contracts
Length of output: 1059
Fix the guarantor zero-address check order in the example.
At line 374, guarantor is checked before it is assigned. Since guarantor is a named return variable (zero-initialized), the condition guarantor == address(0) is always true, making the check redundant and the logic incorrect.
Suggested doc fix
- if (paymasterData.length < 22 || guarantor == address(0)) {
+ if (paymasterData.length < 22) {
return address(0);
}
-
- guarantor = address(bytes20(paymasterData[:20]));
+
+ guarantor = address(bytes20(paymasterData[:20]));
+ if (guarantor == address(0)) {
+ return address(0);
+ }📝 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.
Suggested change
| // Check guarantor data (should be at least 22 bytes: 20 for address + 2 for sig length) | |
| // If no guarantor specified, return early | |
| if (paymasterData.length < 22 || guarantor == address(0)) { | |
| return address(0); | |
| } | |
| guarantor = address(bytes20(paymasterData[:20])); | |
| uint16 guarantorSigLength = uint16(bytes2(paymasterData[20:22])); | |
| // Check guarantor data (should be at least 22 bytes: 20 for address + 2 for sig length) | |
| // If no guarantor specified, return early | |
| if (paymasterData.length < 22) { | |
| return address(0); | |
| } | |
| guarantor = address(bytes20(paymasterData[:20])); | |
| if (guarantor == address(0)) { | |
| return address(0); | |
| } | |
| uint16 guarantorSigLength = uint16(bytes2(paymasterData[20:22])); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/modules/ROOT/pages/paymasters.adoc` around lines 372 - 379, The current
guard checks guarantor == address(0) before guarantor is assigned (it’s the
named zero-initialized return), so change the logic to first validate
paymasterData length, then parse and assign guarantor using guarantor =
address(bytes20(paymasterData[:20])), and only after that check if guarantor ==
address(0) (or simply remove the redundant zero check if you only meant to
early-return on insufficient length); adjust related variables like uint16
guarantorSigLength = uint16(bytes2(paymasterData[20:22])) to remain after the
length check/assignment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates Paymasters from Community Contracts
PR Checklist
npx changeset add)