docs: correct stale documentation and add operator guides - #67
Conversation
|
Branched from Note for reviewers: this describes the assembled behavior of the sibling feature PRs (light-client punishment, genesis round-trip, client authentication, photon fees), so it reads truthfully once those land. Merging it after those avoids a window where docs are ahead of code. |
c6f2086 to
cf9e63f
Compare
tbruyelle
left a comment
There was a problem hiding this comment.
A welcome update to the docs, but with a few hallucinations about command names and some errors that need to be fixed.
|
no hallucinations @tbruyelle, you missed the comment #67 (comment), you are looking at this PR without considering the docs update takes into account all changes of PRs 64 to 78. Will reply to each separately. Only one of your findings is actually correct. Update: replied to each thread with the specific PR each statement comes from. The one correct finding (the fee-pool admission comparison) is fixed in fcda155, together with a rewrite this PR owed anyway: the client-authentication pages now describe the owner-declaration design #65 moved to after this PR was opened. |
cf9e63f to
fcda155
Compare
|
One more content update: #78 folded MsgRetireConsumer into MsgRemoveConsumer per review there, so the lifecycle, runbook, fee-pool, events, and queries pages now describe the single message whose effect depends on the consumer's phase. |
8899478 to
aa375e2
Compare
My bad OK |
- delete the stale fork-era PLAN.old.md and REWRITE_SUMMARY.md and rewrite DESIGN_RATIONALE.md, README.md, and AGENTS.md to match the shipped design (PAUSED phase, provider-side no_valupdates, the fee-pool economics, IBC v2 data flow), and correct docs/consumer-fee-pool.md and consumer-lifecycle.md. - add operator and reference docs: a consumer-launch runbook (with fee-pool funding), a params reference for both modules, key-assignment and equivocation/evidence guides, a validator-obligations overview, a security-model / trust-assumptions doc, and a tests/e2e README. - reflect the shipped punishment and fee behavior: light-client attacks are punished (byzantine signers slashed/jailed/tombstoned, an amnesia attack stops the consumer), and downtime fee exclusion applies to the infraction epoch. - add docs/embedding.md, the host-app wiring checklist for a chain integrating the modules: the provider maccPerms entry (whose omission panics the fee-pool sweep inside BeginBlock at the first consumer deletion and halts the chain), the bank send restriction, the provider staking hooks, governance as the IBC client authority, x/evidence on the provider, the consumer message-filter decorator, the provider consensus-key rotation ante decorator, the no_valupdates substitutions and the ordering constraints, the ibcRouterV2 app-id routes, the fee denom, and the consumer keeper construction order -- each with what breaks and whether the failure is silent or fatal. Say plainly that app/provider demonstrates the full wiring while app/consumer is a reduced reference app, and correct README.md's claim that integration "just means adding the v2 routes". - fix the documented consumer fee-pool address preimage: it is derived from the provider module name (vaasprovider-consumer-fee-pool-<id>), not "provider-", and since NewModuleAddress hashes its input the old string named an unrelated, unprotected account. Tell the reader to read fee_pool_address off the chain instead of deriving it. - drop the governance veto that does not exist: a light-client attack the provider cannot punish stops the consumer terminally, and nothing leaves STOPPED. Also correct the trigger -- it fires on no punishable validator, which covers an all-unbonded byzantine set as well as amnesia. - add docs/events-reference.md and docs/queries-reference.md, and link them plus the previously orphaned security-model.md from the README documentation list. The docs that told operators to "watch the acceptance events" now name them, and ErrDepositTooSmall and ErrSubShareWithdraw are documented. - correct further fee-pool and lifecycle claims: the distribution module account is exempt from the send restriction, so a community-pool spend aimed straight at a pool address is not rejected and lands as unattributable balance; the withdraw escrow cap; the withdraw lock covers PAUSED and DELETED blocks everyone; sweep is available pre-launch; the pool must hold a full epoch fee though only eligible shares are drawn; deletion auto-sweeps the pool and clears six more state items; MsgCreateConsumer requires only chain_id and metadata; MakeConsumerGenesis also seeds the safe-mode threshold and the downtime params; client adoption compares against the last computed set, not the last sent one; fee exclusion applies only to a not-yet-distributed epoch; the previous-downtime-params tolerance runs to evidence-max-age plus the challenge window; and the key-assignment signer rule lives in ValidateBasic. - replace stale line-number citations with symbol references, and fix every CLI example: the subcommand root is vaasprovider/vaasconsumer, not provider/consumer. - document pre-launch consumer retirement and the chain-id release: a new MsgRetireConsumer, signable by the consumer owner or by governance when the owner key is lost, erases a consumer still in REGISTERED or INITIALIZED through the same DeleteConsumerChain teardown, with no STOPPED stopover and no unbonding delay, since no validator ever validated the chain. Cover it in consumer-lifecycle.md (a dedicated section plus the DELETED triggers and the summary table), the launch runbook, the queries/tx table, the events table, and DESIGN_RATIONALE's lifecycle summary. Correct the claim that a deleted consumer's chain id stays reserved forever: deletion now releases it as its last step, so the id is registrable again and consumer-chain reports it empty -- a deleted consumer is identified by consumer_id. Note that a funded pool is swept pro rata to its depositors on retirement, dust to the community pool, so a chain that never launched does not strand its prepaid fees. - document the operator procedure for a provider consensus-key rotation, in key-assignment.md with a pointer from validator-obligations.md: the rotation changes the validator's consumer-side identity only on consumers where it has no assigned consumer key, and for those the provider queues and sends an immediate snapshot instead of waiting for the epoch boundary -- so the node signing key must be swapped at the rotation, since the launch grace period is anchored to spawn time and cannot absorb the misses either way. Say that a rotation onto a key already assigned as some validator's consumer key is refused at transaction admission by the ante decorator, and what that costs on a chain that did not wire it. Spell out where the state lands: assigned keys and their reverse mappings follow the validator, fee bookkeeping follows it regardless of assignment, and downtime acceptance bookkeeping deliberately stays under the pre-rotation address where the validator had no assigned key, because that is the identity the consumer validated under -- so a slash queued before the rotation stays challengeable under the old address. Correct embedding.md, which named the hook's old key-assignment-only migration.
… claim The client-authentication sections described the content-bound adoption that the client-authentication PR replaced with explicit owner declarations: MsgUpdateConsumer's client_id binds the provider side, MsgSetProviderClient pins the consumer side, both validated and permanent, and the consumer genesis no longer creates a client. The security model, launch runbook, lifecycle, key-assignment, and events pages, plus README, AGENTS, and DESIGN_RATIONALE, now describe that design; vaas_client_established is documented as emitted by the pin message handler. Retirement's admission was described as 'the same owner-or-gov admission the fee-pool fund and withdraw messages use', but neither of those is gated that way: funding is permissionless and withdrawal is depositor-gated with a gov-only arm while the consumer is launched or paused. The false comparison is dropped; the admission itself is stated as it is.
PR #78 merged MsgRetireConsumer into MsgRemoveConsumer per review: one message whose effect depends on the consumer's phase. The lifecycle, runbook, fee-pool, events, and queries pages now describe that shape, and the remove event documents its new consumer_phase attribute.
aa375e2 to
680ba62
Compare
|
Reference for a pending edit: once #71's review resolves, security-model.md gains this bullet under "Assumptions and out of scope", recording the residual exposure surfaced there: "A malicious consumer binary can manufacture equivocation evidence against honest validators. Every provider validator validates every consumer, with no opt-in, running the binary the consumer's owner distributes. A malicious binary can fork its own chain at the application layer, making each honest validator sign conflicting data with its real consumer key, once per header, so per-node double-sign protection never triggers. The resulting evidence is indistinguishable from a genuine attack, and through the double-vote path it slashes and tombstones on the provider. [one sentence per #71's outcome — option A: "The light-client path punishes the identified coalition automatically, under the same caveat." — option B: "The light-client path is detection-only for exactly this reason: a confirmed attack is recorded on-chain, and punishing it is a governance decision."] The mitigations are social and model-level: registration carries binary_hash and genesis_hash for validator-side vetting, and the residual risk is accepted MVP posture." |
PR #69 renamed DropRejectedEvidencePacket to ReportRejectedEvidencePacket and dropped the ack-bytes attribute (an IBC v2 error ack is a sentinel constant); this row documents the assembled state, so it follows.
|
Content update driven by #69: it renamed DropRejectedEvidencePacket to ReportRejectedEvidencePacket and dropped the ack-bytes attribute from the vaas_consumer_evidence_rejected event (an IBC v2 error ack is a sentinel constant with no application error). The events-reference row is aligned in 00a146d, since this PR documents the assembled state. |
Documentation-only: corrects everything stale and adds the operator guides the
repo was missing.
Corrections
that no longer exists.
DESIGN_RATIONALE.md,README.md,AGENTS.md: rewritten to match theshipped design — the PAUSED lifecycle phase, the provider-side fee-pool
economics (no cross-chain reward distribution), the IBC v2 data flow, the
no_valupdatesmodules being a provider-side concern, global (notper-consumer) infraction parameters, and the removal of features that were
never implemented here.
docs/consumer-fee-pool.md(distribution is per epoch, not per block),docs/consumer-lifecycle.md(actual required fields), anddocs/consumer-downtime.md(fee exclusion applies to the epoch theinfraction falls in).
New guides
step without which a consumer immediately accrues debt.
and validation.
validator that prunes consumer block data cannot assemble a downtime
challenge.
it punishes: content-bound client adoption and the permanent pin (including
the consumer's bootstrap trust-on-first-use window and governance client
recovery as the only re-key path), the punishment matrix for double-signing,
light-client attacks, and downtime, the fee-escrow model, and the explicit
note that an embedding provider chain must wire
x/evidenceorprovider-native double-signs go unpunished.
breaks if it is omitted and whether the failure is silent or fatal. It leads
with the module-account permission whose absence halts the provider at the
first consumer deletion, and covers the staking hooks, the consumer's
message-filter decorator, governance as the IBC client authority,
x/evidence,and the provider ante decorator that rejects a colliding consensus-key
rotation. The reference apps are explained as reference apps — in particular
the consumer app deliberately has no governance module, which is exactly why
client recovery is unusable there.
with its exact attribute keys, and every provider and consumer query with its
CLI command and its gotchas. Two docs previously told operators to "watch the
acceptance events" without naming one.
Corrections worth calling out
preimage is hashed it produced a completely unrelated address rather than a
near miss — anyone deriving a pool address from the docs would have sent funds
to an account the send restriction does not protect. Corrected, with a pointer
to the queries that return the real address instead of deriving it by hand.
escalation as reversible by a governance veto. No such path exists: nothing
moves a consumer out of
STOPPED. Now documented as terminal, which is thedefensible behavior for a chain proven byzantine.
docs/used the wrong subcommand root. All of them arecorrected and machine-checked against the modules' actual command names.