fix: signed dispatch_permit weight refund + nonce consumption on revert#1468
fix: signed dispatch_permit weight refund + nonce consumption on revert#1468mrq1911 wants to merge 1 commit into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
|
Crate versions that have not been updated:
Runtime version has not been increased. |
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up fix to the signed dispatch_permit path in pallet-transaction-multi-payment, aligning its fee/weight accounting and nonce-consumption behavior with the unsigned path while preserving the signed path’s anti-grief behavior for runner-level rejections.
Changes:
- Forward runner-computed
PostDispatchInfoon signed success to refund/charge based on actual weight (gas used) instead of the declaredgas_limit. - On signed revert (call-execution failure), return
Ok(post_info)to commit nonce consumption and avoid permit replay. - Update signed-permit integration tests to reflect “revert commits + nonce consumed” semantics and to isolate the hard-failure (
Err) case to runner errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pallets/transaction-multi-payment/src/lib.rs |
Adjusts signed dispatch_permit result handling to forward post-info on success and commit nonce on revert. |
integration-tests/src/evm_permit.rs |
Updates signed-permit integration tests to validate new revert/nonce and runner-error behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // RunnerError: nothing charged. Keep Err (`Pays::Yes`) so the paymaster | ||
| // still pays the extrinsic fee — the per-attempt cost that replaces the | ||
| // unsigned-path autopause. Never call `on_dispatch_permit_error()` here. | ||
| Err(e) if e.error == Error::<T>::EvmPermitRunnerError.into() => Err(e.error.into()), | ||
| // Revert already consumed the nonce and charged gas; commit it like the | ||
| // unsigned path. Returning Err would roll the nonce back → replayable. | ||
| Err(e) => Ok(e.post_info), |
|
Quick benchmark at commit abd1dbc has been executed successfully. |
Follow-up to #1462. Two fixes to
do_dispatch_permit_signed, both in the resultmatch:1. Charge the paymaster for gas used, not the full
gas_limitOn success the branch returned
Ok(().into())(actual_weight: None,Pays::Yes), discarding thePostDispatchInfothe runner already computed. The signer was charged a Substrate weight fee against the full declaredgas_limiton top of EVM gas, while the unsigned path forwards the runner's actual weight. Now forwardsOk(post).2. Consume the permit nonce when the inner call reverts
A revert made
dispatch_permitreturnErrafter incrementingNoncesStorage; FRAME's per-extrinsic storage layer then rolled that increment back, leaving the permit replayable — diverging from the unsigned path, which commits (unwrap_or_else→Ok) and burns the nonce on revert.The error arm is now split:
Errwith default post-info, so the paymaster pays the extrinsic fee per attempt. This per-attempt cost is the signed path's anti-grief floor in place of the unsigned-path autopause; the branch still never callson_dispatch_permit_error().Ok(e.post_info), mirroring the unsigned path.Pays::Nowaives the duplicate Substrate fee — the EVM gas is the payment.No double-spend was possible before or after: a successful execution consumes the nonce, so a permit succeeds at most once.
Tests
Updated four signed-permit integration tests to the new semantics (revert →
Ok+ nonce consumed; restore-on-revert; default post-info repointed to theRunnerErrorpath via au64::MAX-signed gas limit). Unsigned-path, success, replay, and two-paymaster-race tests untouched.runtime-integration-tests→evm_permit: 43 passedpallet-transaction-multi-payment: 45 passed