pallets: rustdoc is the API reference, dead variants removed, both gated in CI - #403
Open
ilchu wants to merge 4 commits into
Open
pallets: rustdoc is the API reference, dead variants removed, both gated in CI#403ilchu wants to merge 4 commits into
ilchu wants to merge 4 commits into
Conversation
This was referenced Sep 10, 2026
ilchu
added this pull request to stack #407
September 10, 2026 11:56
Collaborator
|
/aireview |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🤖 AI-generated review — advisory only, not a substitute for human review.
Summary
- Adds comprehensive rustdoc comments across all calls, errors, events, and types in the storage pallets, enforced via
#![warn(missing_docs)]. - Removes unused error and event variants across pallets and regenerates corresponding Subxt and PAPI runtime bindings.
- Introduces
check-dead-variants.shand wires it into CI to guard against unused pallet error and event variants.
bkontur
reviewed
Sep 10, 2026
| BalanceOf<T>, | ||
| >; | ||
|
|
||
| /// v0 → v1: re-encodes every stored `DriveInfo` without its `payment` field. |
Collaborator
There was a problem hiding this comment.
@ilchu I would remove all the migrations at this stage (can be a follow-up)
bkontur
reviewed
Sep 10, 2026
| @@ -1,13 +1,22 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
|
|
|||
| /// Agreement lifecycle: pricing, opening from signed terms, settlement. | |||
Collaborator
There was a problem hiding this comment.
@ilchu does not this duplicate docs in agreements.rs?
bkontur
reviewed
Sep 10, 2026
|
|
||
| for v in $variants; do | ||
| if [ "$kind" = error ]; then | ||
| pattern="Error::(<T>::)?$v([^A-Za-z0-9_]|$)" |
Collaborator
There was a problem hiding this comment.
@ilchu what about Error::<Test> or Error::<Runtime>?
bkontur
reviewed
Sep 10, 2026
| verb="emitted" | ||
| fi | ||
| # shellcheck disable=SC2086 # $sources is a whitespace-separated file list | ||
| if ! grep -qE "$pattern" $sources; then |
Collaborator
There was a problem hiding this comment.
@ilchu I am not sure now, maybe we can have variant that is not directly used in the repo or lib, but can be used with some provided implementation in the Fellows repo level (most probably does not happen)
Every call, error, event, field and runtime-API type in the three pallets now carries a short doc comment written for the end user: what a call does and who may call it, what an error means and how to get past it, what an event reports. The text ships in the runtime metadata, so PAPI descriptors, subxt bindings and polkadot.js Apps all show the same words. `#![warn(missing_docs)]` on each pallet crate turns a missing doc into a CI failure, since clippy runs with `-D warnings`. Four intra-doc links to private items and one unescaped generic in a module doc are fixed so `cargo doc` is clean with warnings denied.
Seven storage-provider errors, five storage-provider events and one drive-registry error were declared but never used. Four errors and three events had been dead since the initial commit; the rest lost their last use when agreement negotiation moved off-chain and when the challenge flow was overhauled. rustc never flagged them: the enums are exported API, and FRAME's macros reference every variant, so `dead_code` does not apply. Each one still occupied a slot in the runtime metadata and a type in every client's generated bindings. Removed errors: BucketNotFrozen, NotBucketMember, ChallengeAlreadyExists, InvalidChallengeProof, LeafBeyondCanonical, InvalidDeletionProof, InvalidMultiaddr, NotAuthorizedToShare. Removed events: ProviderAddedToBucket, PrimaryAgreementEndedEarly, AgreementAccepted, AgreementOwnershipTransferred, AgreementExpiredClaimed. Error and event indices shift; the bindings are regenerated in the next commit.
Refreshed from a local paseo dev chain after the variant removal, so the generated error and event enums match the runtime again. The pallet rustdoc now travels with the metadata as well, so the generated Rust and TypeScript carry the same doc comments as the pallets.
rustc cannot flag them: the enums are exported API and FRAME's macros reference every variant. scripts/check-dead-variants.sh parses the `#[pallet::error]` and `#[pallet::event]` blocks and greps each pallet's production sources for a use; the new `check-dead-variants` job runs it whenever Rust files change and gates `basic-checks`.
ilchu
force-pushed
the
ic/pallet-rustdoc-gate
branch
from
September 10, 2026 12:47
7f1bcf3 to
da1ba0d
Compare
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
2 participants
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.
Rustdoc is the API reference
Every call, error, event, field and runtime-API type in the three pallets now carries a short doc comment written for the end user: what a call does and who may call it, what an error means and how to get past it, what an event reports. That text ships in the runtime metadata, so the PAPI descriptors, the subxt bindings and polkadot.js Apps all show the same words. The pallets set
#![warn(missing_docs)], and since the clippy job already runs with-D warnings, an undocumented public item now fails CI. Four intra-doc links to private items and one unescaped generic in a module doc are fixed socargo docis clean with warnings denied.This is the mechanical replacement for the hand-written reference docs, which #406 deletes. The review thread on #372 asked whether those docs were worth keeping; they had described seven extrinsics removed in June and missed the two that replaced them, so the answer was no.
Dead variants
Thirteen error and event variants were declared but never raised or emitted. Seven had been dead since the initial commit; the rest lost their last use when negotiation moved off-chain (#105) and when the challenge flow was overhauled (#125). rustc cannot flag these: the enums are exported API and FRAME's macros reference every variant, so
dead_codenever applies. They are removed and the subxt and PAPI bindings regenerated from a local paseo dev chain. Error and event indices shift, which is why the bindings commit sits right after the removal.Two of the removed events,
AgreementOwnershipTransferredandProviderAddedToBucket, were placeholders for features that do not exist. The first comes back with a real emitter in #405. The second belongs to the multi-primary join path that #105 removed and that the design still describes; that is parked and tracked separately.CI check
scripts/check-dead-variants.shparses the error and event blocks of each pallet and greps its production sources for a use of every variant; the newcheck-dead-variantsjob runs it whenever Rust files change and gatesbasic-checks. No allowlist: a variant lands in the PR that uses it.