feat(storage-provider): track lifetime revenue per provider - #397
feat(storage-provider): track lifetime revenue per provider#397kelwinshen wants to merge 4 commits into
Conversation
Adds ProviderStats.lifetime_revenue, a monotonically increasing record of all payment ever received by a provider: agreement settlements, extension payments, replica sync payments, and challenger-cost reimbursement for successfully defended challenges. Never resets, not even on a slash -- it's a historical record, not a live balance. Exposed through ProviderInfoResponse (query_provider_info, query_providers, and the marketplace matching queries) so UIs/indexers can read it without replaying event history themselves. Discussed in paritytech#386: a cheap, useful stat the chain can track directly instead of requiring an off-chain indexer.
|
User @kelwinshen, please sign the CLA here. |
|
@kelwinshen thank you for the contribution 🎉 :), please, update the relevant struct in |
|
/aireview |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI-generated review — advisory only, not a substitute for human review.
Summary
- Adds
lifetime_revenuetracking toProviderStatsand exposes it across provider query APIs. - Updates revenue totals on agreement completions, extensions, replica synchronizations, and challenge completions.
Correctness
- Missing storage migration for
ProviderStatsfield addition- Impact: Adding
lifetime_revenuetoProviderStatsalters the SCALE-encoded layout of stored provider data. On any existing deployed network or persistent testnet, reading existingProvidersstorage entries will fail to decode unless a migration is run. Verify if the pallet is deployed to live state requiring a storage version upgrade. - Fix: Implement a pallet storage migration that decodes previous
ProviderInfoentries and setslifetime_revenueto zero, and bump the palletStorageVersionincrates/pallets/storage-provider/src/lib.rs.
- Impact: Adding
@kelwinshen please, ignore, no migration is needed at this stage |
bkontur
left a comment
bkontur
left a comment
There was a problem hiding this comment.
@kelwinshen please check #397 (comment) at least and sign the CLA stuff
Keeps the design doc in sync with the ProviderStats struct after adding lifetime_revenue in the previous commit, per review feedback on paritytech#397.
danielbui12
left a comment
danielbui12
left a comment
There was a problem hiding this comment.
since you've made some changes to runtime, you need to re-generate papi descriptor (packages/papi) and storage-subxt (crates/storage-subxt). checkout justfile for useful cmd.
I believe some parts of provider-node may need to be changed as well.
ilchu
left a comment
ilchu
left a comment
There was a problem hiding this comment.
Looks pretty clean to me. Sure, it's a bit clanky to remember doing revenue accounting on every provider settlement and it also might be error prone but I don't see any immediate issues.
Splits ProviderInfoResponse's historical counters (agreements_total, challenges_failed, lifetime_revenue, etc.) into a nested ProviderStatsInfo, separate from the provider's current settings and connection info. Matches how the on-chain ProviderInfo already nests stats, and keeps the response from growing as one long flat list as more stats get added. Updates every construction site (queries.rs, marketplace.rs) and consumer (the storage-client SDK, drive-ui, s3-ui) to match. Also adds lifetime_revenue to the SDK's own ProviderInfo struct, which never exposed it. Regenerates storage-subxt and the papi descriptors against the new shape. Along the way, found and fixed a stale test fixture in provider-coordinator that was missing lifetime_revenue in its hand-built ProviderInfo mock, left over from when the field was first added on-chain. Requested in review on paritytech#397 by bkontur and danielbui12.
|
Regenerated storage-subxt and papi against the new shape, updated drive-ui and s3-ui, ran their actual build to confirm. Checked provider-node and provider-coordinator too, they don't touch ProviderInfoResponse so nothing to change there. Did find one stale test mock in provider-coordinator missing lifetime_revenue from way earlier, fixed that as well. |
| "metadata": ".papi/metadata/parachain.scale", | ||
| "genesis": "0x4c9b8938deefded9bbafe8a3c0f8b7d91b47d83833ca74542d4aa4df87d81859", | ||
| "codeHash": "0x39a2a7d0e6d0aca1b9cdc021518f7341a57c56ea25ebac287b5983b5b2e84ff3" | ||
| "genesis": "0x2c636f36f0ab07ee4d5d264dc18c5365865779902221192d2f5af21da9ba9653", |
There was a problem hiding this comment.
I remember we always try to pin genesis hash following paseo runtime, is this paseo runtime genesis hash?
otherwise, try
just start-paseo-chain
just subxt-codegen
| /// Total payment ever received by this provider for storage service: | ||
| /// agreement settlements, extension payments, and replica sync | ||
| /// payments. Monotonically increasing, never reset by a slash or |
There was a problem hiding this comment.
Should also mention challenger cost reimbursement on defended challenges though, both here and in the actual struct itself.
…time_revenue lifetime_revenue also grows from the challenger's cost share on a successfully defended challenge (respond_to_challenge), not just agreement/extension/replica payments. The comment only listed the first three sources. Requested in review on paritytech#397 by ilchu.
4 participants
Adds
lifetime_revenuetoProviderStats, a running total of all payment a provider has ever received: agreement settlements, extension payments, replica sync payments, and challenger-cost reimbursement when a challenge is successfully defended. It never resets, not even on a slash, since it's meant to be a historical record rather than a live balance.It's exposed through
ProviderInfoResponse(query_provider_info,query_providers, and the marketplace discovery queries) so a UI or indexer can read it directly instead of replaying event history.Discussed in #386, where eskimor agreed a cheap stat like this is worth adding to runtime state directly rather than relying on off-chain indexers.