-
Notifications
You must be signed in to change notification settings - Fork 52
Change Fee Handler #1869
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: develop
Are you sure you want to change the base?
Change Fee Handler #1869
Conversation
Neopallium
left a comment
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.
Looks good.
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.
Pull request overview
This PR updates Polymesh fee-handling and identity authorization retry behavior so that certain authorization-based extrinsics can have their retry counters decremented when the extrinsic fails.
Changes:
- Extends
CddAndFeeDetailsto track authorization IDs and decrement authorization retry counts on failed dispatch. - Updates
ChargeTransactionPaymentto capture anauth_idinpre_dispatchand decrement onpost_dispatchfailure. - Introduces a new
MaxAuthRetriesruntime constant and uses it to initializeAuthorization.count, withget_non_expired_authnow requiringcount > 0.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| primitives/src/traits.rs | Extends CddAndFeeDetails with payer-context helpers plus authorization-id/retry hooks; changes get_valid_payer signature to take AccountId by value. |
| pallets/transaction-payment/src/lib.rs | Tracks auth_id in signed extension Pre and decrements authorization retry count when dispatch fails. |
| pallets/runtime/tests/src/storage.rs | Updates test runtime to implement new CddAndFeeDetails methods and adds MaxAuthRetries. |
| pallets/runtime/tests/src/staking/mock.rs | Updates staking mock runtime to wire MaxAuthRetries and new CddAndFeeDetails signatures. |
| pallets/runtime/tests/src/relayer_test.rs | Updates test callsites for get_valid_payer signature change. |
| pallets/runtime/tests/src/identity_test.rs | Updates test callsites for get_valid_payer signature change. |
| pallets/runtime/tests/src/fee_details.rs | Updates test callsites for get_valid_payer signature change (clones/moves). |
| pallets/runtime/testnet/src/runtime.rs | Wires MaxAuthRetries into the runtime’s pallet_identity::Config. |
| pallets/runtime/mainnet/src/runtime.rs | Wires MaxAuthRetries into the runtime’s pallet_identity::Config. |
| pallets/runtime/develop/src/runtime.rs | Wires MaxAuthRetries into the runtime’s pallet_identity::Config. |
| pallets/runtime/common/src/fee_details.rs | Refactors CddHandler logic and implements authorization-id extraction + decrement hook. |
| pallets/identity/src/lib.rs | Adds new Config associated constant MaxAuthRetries. |
| pallets/identity/src/auth.rs | Initializes Authorization.count from MaxAuthRetries, requires count > 0 for get_non_expired_auth, and adds a decrement helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[pallet::constant] | ||
| type MaxGivenAuths: Get<u32>; | ||
|
|
||
| /// Maximum number of authorizations an identity can give. |
Copilot
AI
Jan 23, 2026
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.
The doc comment for MaxAuthRetries is a copy of MaxGivenAuths and is misleading. It should describe that this constant controls the max number of retry attempts (i.e., how many failed attempts are allowed before an authorization is considered unusable), not the number of authorizations an identity can give.
| /// Maximum number of authorizations an identity can give. | |
| /// Maximum number of retry attempts allowed for an authorization | |
| /// before it is considered unusable. |
| ); | ||
| } | ||
|
|
||
| Ok(Some(caller_acc_id.clone())) |
Copilot
AI
Jan 23, 2026
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.
caller_acc_id is already owned here, so cloning it just to wrap in Some is unnecessary. Returning Ok(Some(caller_acc_id)) avoids an extra clone of AccountId.
| Ok(Some(caller_acc_id.clone())) | |
| Ok(Some(caller_acc_id)) |
other
Authorization.count] every time the any of the following extrinsics fail: