fix: arm safe mode for never-connected consumers and harden snapshot and photon fee edges - #66
Open
giunatale wants to merge 1 commit into
Conversation
giunatale
requested review from
clockworkgr,
julienrbrt and
tbruyelle
as code owners
July 30, 2026 15:56
Contributor
Author
|
Branched from Note for reviewers: two statements in this PR's |
giunatale
force-pushed
the
giunatale/feat/client-authentication
branch
from
July 31, 2026 18:40
2b71a23 to
92ea64b
Compare
giunatale
force-pushed
the
giunatale/fix/safe-mode-and-photon-edges
branch
from
July 31, 2026 18:40
1ce861e to
39a5b5c
Compare
…hoton fee edges A NewChain consumer now arms the VSC staleness clock at InitGenesis with the genesis block time, so a consumer whose relayer never materializes crosses SafeModeThreshold and enters restricted mode instead of running on its genesis validator set unrestricted forever (the unset clock falls back to the current block time and never reads stale). Restarts still restore the exported value, which is now always present for chains that launched as NewChain; PreVAAS chains stay unarmed until their first VSC, since the standalone staking keeper still runs the chain there. OnRecvVSCPacketV2 now rejects, with an error acknowledgement and before any state change, a packet whose application would leave the consumer with zero validators, the state in which the EndBlock flush halts the chain irrecoverably. Snapshots are judged by their own positive-power updates (they replace the set wholesale); diffs by overlaying the current cross-chain set with the accumulated pending changes and the packet's updates. Detecting the diff case reuses the existing accumulate helpers, so a diff removing every validator is guarded too, not only the snapshot resync path; empty heartbeat diffs over a live set keep flowing. PhotonFeeDecorator keeps deriving the expected voucher denom from the live provider client pin: the pin moves at most once, from the unroutable genesis client to the first VSC-delivering client, and ICS-20 vouchers can only arrive over that adopted client, so the derived denom tracks exactly what real vouchers carry (a static genesis-time param could never know the adopted client id). The decorator now splits its behavior at routability of the pin: before the pin is routable no voucher can exist, so it stays a full no-op -- rejecting the fee-less relayer traffic of that phase would block the first VSC from ever arriving -- and once routable it enforces that fees are non-empty and denominated in the voucher, closing the empty-fee bypass of the photon-only policy. Gas-estimation simulations are exempt from the non-empty requirement. Whether that policy applies at all is a new consumer param, photon_fees_enabled (field 8 of ConsumerParams, default false), so every node derives the decision from consensus state. The decorator has no CheckTx-only carve-out and therefore also rejects during FinalizeBlock, so a node-local switch would let two validators configured differently disagree on whether a transaction is valid and diverge on the app hash, with a validator joining after launch on the shipped default as a mundane trigger. Only the on/off decision lives in params; the expected voucher denom stays derived from the live pin, which no genesis-time value could name. A chain opts in through its consumer genesis or a governance MsgUpdateParams, which carries the flag through unchanged -- unlike the provider-owned downtime window fields, which that handler keeps preserving from the stored params. The reference consumer app wires the decorator unconditionally, immediately before fee deduction per the decorator's contract: insertion is app wiring and cannot depend on per-block state, so the decorator self-gates on the param and no-ops on chains that did not opt in. DisabledModulesDecorator and MsgFilterDecorator were already wired. The stale claim in docs/consumer-liveness.md that a fresh consumer is never stale before its first VSC is updated to the armed-at-genesis behavior. Four further corrections to that same doc, none behavioral: the headline packet-timeout claim now states the real 1h VaasTimeoutPeriod default instead of reading the 24h MaxTimeoutDelta ceiling as the effective timeout (the argument that a timeout is too weak a removal signal only gets stronger at an hour); the snapshot trigger lists all three conditions QueueVSCPackets tests rather than the acked/sent comparison alone, adding the locally queued packet and the empty stored consumer valset; the liveness query example uses the vaasprovider subcommand root; and last_ack_time is described as the query's one piece of stored state, with the other three fields derived from it. Tests: unit coverage for the genesis arming and the unarmed restart fallback, for empty-snapshot and set-emptying-diff rejection with state left untouched, and for the photon decorator across the param gate and both pin phases, including the simulation carve-out; for the param's default, its validity under either setting, the keeper accessor the decorator reads, its genesis round-trip and its governance toggle; and new app-level ante tests that run signed transactions through the full reference ante chain against real keepers, driving the policy from genesis params over routable and unroutable pins. Existing tests encoding the removed behaviors changed deliberately: TestSnapshotEmptyRemovesAll became TestEmptySnapshotRejected, the heartbeat-based debt and downtime-staging tests now seed a live validator set, and the photon ante table drops its empty-fee-passes case. No photon e2e is included: enforcing photon fees on the shared e2e consumer would break every other subtest's fee payments, and the harness has never exercised an ICS-20 v2 transfer relay to mint real vouchers; at the param's default the decorator no-ops, so the suites are unaffected by the wiring.
giunatale
force-pushed
the
giunatale/feat/client-authentication
branch
from
July 31, 2026 19:56
92ea64b to
021c4bd
Compare
giunatale
force-pushed
the
giunatale/fix/safe-mode-and-photon-edges
branch
from
July 31, 2026 19:56
39a5b5c to
3c074c0
Compare
julienrbrt
approved these changes
Aug 6, 2026
| // fatal as an empty snapshot) is guarded rather than only the snapshot resync | ||
| // path. An empty-update heartbeat diff over a live set stays accepted: it | ||
| // leaves the set as it was, which is non-empty. | ||
| func (k Keeper) wouldEmptyValidatorSet(ctx sdk.Context, newChanges vaastypes.ValidatorSetChangePacketData) bool { |
Member
There was a problem hiding this comment.
tbf, i do not see the use case of this. the validator set of a consumer and the provider does not diverge (as we do not support PSS), so this cannot trigger. I guess it is fine to leave it here but just noting.
julienrbrt
approved these changes
Aug 6, 2026
| } | ||
|
|
||
| providerClientID, ok := d.keeper.GetProviderClientID(ctx) | ||
| if !ok || !d.keeper.HasRoutableProviderClient(ctx) { |
Member
There was a problem hiding this comment.
Can we add an e2e case for this? I do not believe we actually have any txs being sent from the consumer.
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.
Four consumer-side hardening items.
Never-connected consumers now enter safe mode
LastVSCRecvTimewas only ever set on packet receipt, and the getter fell backto the current block time when unset — so a consumer that never received a
validator-set update was never considered stale and never entered safe mode.
InitGenesisnow arms the clock at genesis block time for a new chain, soSafeModeThresholdstaleness accrues from chain start. An explicit timestampfrom a restart genesis still wins, and every accepted update still re-arms the
clock. (A pre-VAAS standalone chain deliberately stays unarmed until its first
update: a live standalone economy must not enter restricted mode
mid-changeover.) The liveness documentation asserted the old behavior and was
corrected.
A packet can no longer empty the validator set
A well-formed packet whose application would leave the consumer with an empty
validator set halts the chain at EndBlock.
OnRecvVSCPacketV2now rejects suchpackets with an error acknowledgement before any state write — for snapshots
(the packet's positive-power updates are the entire resulting set) and for
diffs (the current set overlaid with pending changes plus the packet,
reproducing exactly what EndBlock would apply). Rejection happens inside the
IBC cache context, so no state leaks; out-of-order stale packets still
deduplicate to a success acknowledgement first, preserving replay semantics.
Heartbeat diffs over a live set are unaffected.
Photon fee decorator: bypass closed, bootstrap made explicit
The expected fee denom stays derived from the pinned provider client
(
ibc/SHA256("transfer/<clientID>/uphoton")): under client authentication thepin moves at most once at bootstrap, and IBC vouchers can only ever arrive over
the adopted, routable client — so the derived denom tracks exactly the denom
real vouchers carry, while a static genesis-time parameter could never name the
adopted client id. The decorator godoc documents this.
Behavior now splits on pin routability:
exist yet, so there is nothing to police, and rejecting the fee-less relayer
traffic of that phase would block the first validator-set update forever. The
message-filter decorator remains the traffic restriction for that phase.
AND the fee must be non-empty — the empty-fee bypass is closed. Gas
simulation is exempt from the non-empty check only; a wrong denom still
rejects in simulation.
The decorator is wired, not shipped inert
PhotonFeeDecoratoris now always part of the reference consumer app's antechain, inserted immediately before fee deduction, and the on/off decision is a
consumer module parameter (
photon_fees_enabled, default false) rather thannode-local configuration.
That distinction is the point. The decorator rejects transactions in
FinalizeBlock, not only inCheckTx, so it is consensus-relevant: driving itfrom
app.tomlwould let two validators disagree on whether a transaction isvalid, diverging the app hash and halting the chain — and the trigger would be
as mundane as a validator joining after launch with the shipped default. As a
parameter it comes from state, so every node agrees, it round-trips through
genesis, and governance can flip it through the consumer's existing
MsgUpdateParams. When it is false the decorator is a no-op.New app-level tests build the real consumer app once per parameter setting and
run fully signed transactions through the real ante chain against real keepers:
enabled + voucher denom accepted and deducted; enabled + wrong denom rejected
(from an unfunded account, proving the rejection is policy, not funds); enabled
A photon-fees e2e is deliberately not included: enabling it on a shared e2e
consumer would make every other subtest's fee payments invalid, and acquiring a
real voucher requires an ICS-20 v2 transfer relay the harness has never
exercised. The app-level tests own the policy; at the parameter's default the
decorator no-ops, so the e2e suites are unaffected by the wiring.
Testing
Unit + app-module suites green (including the app module's first ante tests);
golangci-lintclean; both Docker e2e suites pass, including the livenesssuite's safe-mode scenarios with the genesis-armed clock.