Adds AllowanceBasedPriority extension#448
Conversation
This extension gives `store` transactions a proportional priority boost based on the already consumed allowance.
Re-authorizing an unexpired account/preimage replaces `bytes_allowance` rather than adding to it. Three tests assumed the additive form. Renamed `authorize_account_does_not_push_expiry` to `re_authorize_account_replaces_allowance_and_keeps_expiry` to reflect both invariants it now covers.
`AllowanceBasedPriority` is now generic over a `BoostStrategy` so the runtime can pick its policy in the `TxExtension` tuple. Two impls ship: - `ProportionalBoost`: original PR #448 behaviour — linear in remaining allowance. - `FlatBoost`: constant boost while in-budget, `0` once over-budget. The proportional form is vulnerable to censorship: a fresh-allowance signer outranks partly-used ones and can starve them with small-tx spam (see karolk91's analysis on PR #448). `FlatBoost` makes in-budget signers all rank equal and over-budget ones strictly lower; pool nonce/ arrival ordering breaks ties. Both runtimes are wired to `FlatBoost`. Three pallet unit tests (`proportional_scales_with_remaining_allowance`, `flat_is_constant_while_in_budget`, `flat_does_not_let_fresh_outrank_partly_used`) document the contracts and the censorship-relevant difference.
|
@karolk91 @franciscoaguirre — please take a look at this PR. I’ve tried to minimally align it with https://github.com/paritytech/individuality/pull/785, based on the triangle design from https://github.com/paritytech/triangle-js-sdks/pull/129. Please review it — I’d like to merge both this PR and its counterpart (https://github.com/paritytech/individuality/pull/785) and deploy everything to Paseo for testing next week. This will unblock at least I also added a boost to the transaction count to prevent spam with 1B transactions, as you both requested 🙂 — I borrowed that part from Cisco’s PR: #469. I’ve prepared changes for the hard cap and renewals as well, but after our discussion I need to think them through more tomorrow: #457. |
| // Only `store` / `store_with_cid_config` get the boost. `renew` also carries | ||
| // `Origin::Authorized` and does consume allowance, but it operates on | ||
| // already-stored data and shouldn't compete for the same priority slots as | ||
| // new submissions. |
There was a problem hiding this comment.
I actually argue renewals should have the same priority. We can change it in a future PR though after discussion
|
|
||
| /// Renew calls wrapped in utility/sudo require authorization, same as store. | ||
| /// `renew` is allowed as a direct extrinsic (consuming the caller's account allowance) | ||
| /// but rejected when nested inside utility/sudo wrappers. |
There was a problem hiding this comment.
We might want to allow this later for batches
Brings just the AuthorizationExtent + boost-strategy parts of #469; the period / two-slot / bytes_permanent surface is left out. - AuthorizationExtent: gain transactions_used / transactions_allowance fields. authorize_account now uses its 3rd parameter to set/extend transactions_allowance; check_authorization saturating-bumps transactions_used on consume. - BoostStrategy: a shared in_budget helper gates both the byte and tx axes; FlatBoost is in-budget at-cap (matching #469); ProportionalBoost scales by the tighter of the two remainders. - AllowanceBasedPriority::validate increments transactions_used in the post-this-tx extent before computing the boost. - v1->v2 migration carries the old transaction-count field over to the new transactions_allowance. - Genesis config tuple becomes (account, transactions_allowance, bytes_allowance); preimage authorize_* always sets transactions_allowance = 1. - Tests + bulletin-{westend,paseo} runtime tests updated for the new expected counters; new boost_tests::tx_axis_gates_boost_independently. Out of scope (per #469): bytes_permanent, the two-slot current/next grant model, for_period parameter, renew matching in the boost extension, the People-Chain-aligned period model. Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
a49453b to
1d0f322
Compare
Per review feedback (#448 r3163274071): resetting `bytes` and `transactions` on refresh implicitly grants a fresh boost-tier window, which conflates two operations under one extrinsic. Refresh now does exactly what its name says — extend the expiration block. Holders who want more capacity call `authorize_account` (additive on the unexpired path). Doc updates on `refresh_account_authorization`, `refresh_preimage_authorization` and the `refresh_authorization` helper make the split explicit. `authorize_account`'s doc also gets updated to describe the new tx-axis additivity.
- `AuthorizationPeriod` 90 days -> 14 days, aligning with the `LongTermStoragePeriodDuration` (2 weeks) on the People-Chain side. - Drop the unused `SudoPriority` and `SetPurgeKeysPriority` constants; they were only chained into `RemoveExpiredAuthorizationPriority` and not wired anywhere else. - `RemoveExpiredAuthorizationPriority` is now `TransactionPriority::MAX` so permissionless cleanups always outrank stores. - `StoreRenewPriority` is `TransactionPriority::MAX / 4`. With `AllowanceBasedPriority` adding `ALLOWANCE_PRIORITY_BOOST` for in-budget signers, in-budget txs land just above over-budget ones without saturating `u64` and with plenty of headroom both above generic transactions and below the cleanup ceiling.
* Adds `AllowanceBasedPriority` extension This extension gives `store` transactions a proportional priority boost based on the already consumed allowance. * Nit plus test * Do not degrade user's performance in time - reset usage when refresh_authorization * Migration for Paseo * Fix migration * fmt * Ping CI * Adjust `AuthorizationExtent` to keep bytes/bytes_permanent/bytes_allowance * Add MaxPermanentStorageSize Config bound Introduces a chain-wide cap on bytes committed to permanent storage (via `renew`) so each runtime can express its own permanent-storage budget. The bulletin-westend runtime sets it to 1.7 TiB, the test mock leaves it unbounded. * Fix tests to match assignment semantics of `authorize` Re-authorizing an unexpired account/preimage replaces `bytes_allowance` rather than adding to it. Three tests assumed the additive form. Renamed `authorize_account_does_not_push_expiry` to `re_authorize_account_replaces_allowance_and_keeps_expiry` to reflect both invariants it now covers. * Compilation for paseo after main merge * Doc nit for refresh_authorization * Migration for paseo, which was merged meantime * Refactor allowance boost behind `BoostStrategy`, default to `FlatBoost` `AllowanceBasedPriority` is now generic over a `BoostStrategy` so the runtime can pick its policy in the `TxExtension` tuple. Two impls ship: - `ProportionalBoost`: original PR #448 behaviour — linear in remaining allowance. - `FlatBoost`: constant boost while in-budget, `0` once over-budget. The proportional form is vulnerable to censorship: a fresh-allowance signer outranks partly-used ones and can starve them with small-tx spam (see karolk91's analysis on PR #448). `FlatBoost` makes in-budget signers all rank equal and over-budget ones strictly lower; pool nonce/ arrival ordering breaks ties. Both runtimes are wired to `FlatBoost`. Three pallet unit tests (`proportional_scales_with_remaining_allowance`, `flat_is_constant_while_in_budget`, `flat_does_not_let_fresh_outrank_partly_used`) document the contracts and the censorship-relevant difference. * Docs nits * Compute allowance boost against post-this-tx state * Compute allowance boost against post-this-tx state * Updated soft allowance design * Updated hard allowance design and renewals (draft, TODO: challange with auto-renewal) * Soft-side lifecycle: preserve bytes_permanent across re-authorize and remove_expired Two paired rules from the authorizations design were missing in code: 1. `authorize_account` on an expired-but-present entry now re-grants the cap and resets `bytes` only — it preserves `bytes_permanent`. Previously the expired branch overwrote the whole extent with a fresh one, zeroing `bytes_permanent` and silently bypassing the per-account hard cap on the next renew cycle. 2. `remove_expired_authorization` now refuses with `Error::AuthorizationHasPermanentStorage` while `bytes_permanent > 0`. Removing the entry would orphan the (lazy) ledger drain — its decrement would have nowhere to land. The entry becomes removable once `bytes_permanent` has dropped back to `0` naturally. Together these guarantee the ledger drain (introduced when the hard-side work lands) is the only path that ever decrements `bytes_permanent`. No other code path can clobber or orphan the counter. Tests cover both rules: `authorize_account_after_expiry_preserves_bytes_permanent` and `remove_expired_account_authorization_refuses_while_bytes_permanent_outstanding`. * paseo: back MaxPermanentStorageSize with `parameter_types! { pub storage }` Replaces the compile-time `MAX_PERMANENT_STORAGE_SIZE` const with a storage-backed `parameter_types!` entry. The Config trait constant is unchanged on the pallet side; the runtime just chooses a mutable backing, so governance (root) can raise or lower the cap at runtime via `system.set_storage` without a runtime upgrade. Initial value is still 1.7 TiB, seeded on first read. * WIP: hard cap (very draft) * Minimalistic alignment with paritytech/individuality#785 (part 1) * Pick transactions_used / transactions_allowance from #469 Brings just the AuthorizationExtent + boost-strategy parts of #469; the period / two-slot / bytes_permanent surface is left out. - AuthorizationExtent: gain transactions_used / transactions_allowance fields. authorize_account now uses its 3rd parameter to set/extend transactions_allowance; check_authorization saturating-bumps transactions_used on consume. - BoostStrategy: a shared in_budget helper gates both the byte and tx axes; FlatBoost is in-budget at-cap (matching #469); ProportionalBoost scales by the tighter of the two remainders. - AllowanceBasedPriority::validate increments transactions_used in the post-this-tx extent before computing the boost. - v1->v2 migration carries the old transaction-count field over to the new transactions_allowance. - Genesis config tuple becomes (account, transactions_allowance, bytes_allowance); preimage authorize_* always sets transactions_allowance = 1. - Tests + bulletin-{westend,paseo} runtime tests updated for the new expected counters; new boost_tests::tx_axis_gates_boost_independently. Out of scope (per #469): bytes_permanent, the two-slot current/next grant model, for_period parameter, renew matching in the boost extension, the People-Chain-aligned period model. Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com> * transactions_used -> transactions * Refresh: only extend expiry, do not reset consumed counters Per review feedback (#448 r3163274071): resetting `bytes` and `transactions` on refresh implicitly grants a fresh boost-tier window, which conflates two operations under one extrinsic. Refresh now does exactly what its name says — extend the expiration block. Holders who want more capacity call `authorize_account` (additive on the unexpired path). Doc updates on `refresh_account_authorization`, `refresh_preimage_authorization` and the `refresh_authorization` helper make the split explicit. `authorize_account`'s doc also gets updated to describe the new tx-axis additivity. * Move back to 14 * Tighten priority constants for store / renew - `AuthorizationPeriod` 90 days -> 14 days, aligning with the `LongTermStoragePeriodDuration` (2 weeks) on the People-Chain side. - Drop the unused `SudoPriority` and `SetPurgeKeysPriority` constants; they were only chained into `RemoveExpiredAuthorizationPriority` and not wired anywhere else. - `RemoveExpiredAuthorizationPriority` is now `TransactionPriority::MAX` so permissionless cleanups always outrank stores. - `StoreRenewPriority` is `TransactionPriority::MAX / 4`. With `AllowanceBasedPriority` adding `ALLOWANCE_PRIORITY_BOOST` for in-budget signers, in-budget txs land just above over-budget ones without saturating `u64` and with plenty of headroom both above generic transactions and below the cleanup ceiling. * Nit * More nits * Update design * Nit * Hard-side hardening: try_state invariants, validate-time guard, capacity events - Doc: Hard Limit header reflects implementation (was "Proposed; not yet wired"). - try_state: assert PermanentStorageUsed == Σ bytes_permanent across Authorizations == Σ size across PermanentStorageLedger entries; cursor <= current_block + 1; no ledger entries before cursor; used <= MaxPermanentStorageSize. - check_authorization_expired: reject remove_expired_* at validate when bytes_permanent > 0 (new AUTHORIZATION_HAS_PERMANENT_STORAGE = Custom(7)), mirrors the dispatch-time guard so pool ingress doesn't accept soon-to-fail txs. - Events: PermanentStorageUsedUpdated { used } on every renew bump and every drain decrement; PermanentStorageNearCap { used, cap } on rising-edge crossing of PERMANENT_STORAGE_NEAR_CAP_PERCENT (80%). Single helper update_permanent_storage_used owns the read, write, and event emission. 9 new tests cover: remove_expired validate guard (account + preimage); renew/drain event emission; rising-edge near-cap behaviour; happy-path + 4 corruption variants of the new try_state invariants. * Add note * Clippy a nits * Nit for bytes_permanent * nit * Simplify renewal hard cap (#486) * feat: simplify * Fix migration * Drop expired authorizations --------- Co-authored-by: Branislav Kontur <bkontur@gmail.com> * Nit * More nits * Fix migration? --------- Co-authored-by: Bastian Köcher <info@kchr.de> Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
### Problem `dotns bulletin authorize` reports `Authorization was finalized but not applied` even when the on-chain call succeeds. The misleading error `may not have Authorizer privileges` fires every time, blocking CI deploys. Example in [mark3t CI](https://github.com/paritytech/mark3t/actions/runs/25509126698/job/74863965404?pr=209). ### Root cause After paritytech/polkadot-bulletin-chain [#448](paritytech/polkadot-bulletin-chain#448), AuthorizationExtent fields `transactions` and `bytes` now mean consumed counters, not allowances. New fields `transactions_allowance` and `bytes_allowance` carry the granted total instead. ```rust // pallets/transaction-storage/src/lib.rs after #448 pub struct AuthorizationExtent { pub transactions: u32, // CONSUMED so far pub transactions_allowance: u32, // total ALLOWANCE (new field) pub bytes: u64, // CONSUMED so far pub bytes_permanent: u64, pub bytes_allowance: u64, // total ALLOWANCE (new field) } ``` The CLI compares the consumed counter (0 for a fresh authorization) against the requested allowance (1_000_000 default), so 0 >= 1_000_000 is false → "not applied". The error fires only when on-chain dispatch already succeeded (event.ok === true in the finalized case handler). The "Authorizer privileges" wording is a misleading hardcoded message - the actual cause is the wrong field read. ### Fix Read `transactions_allowance` and `bytes_allowance` from `AuthorizationExtent`, update `AuthorizationStatus` type to expose them, fix the comparison in the post-flight verification and pre-flight quota check. JSON output field names kept stable for backwards-compat. ### Test plan - bunx tsc --noEmit clean - bun run lint clean - bun run test:unit — 163/163 passing - Metadata refresh applied; .papi/metadata/bulletin.scale includes `transactions_allowance`, `bytes_allowance`, `bytes_permanent`
This extension gives
storetransactions a proportional priority boost based on the already consumed allowance.TODO