Skip to content

refactor: remove dead fork-era code and make VAAS naming consistent - #76

Open
giunatale wants to merge 6 commits into
giunatale/feat/offline-detectionfrom
giunatale/refactor/cleanup-naming
Open

refactor: remove dead fork-era code and make VAAS naming consistent#76
giunatale wants to merge 6 commits into
giunatale/feat/offline-detectionfrom
giunatale/refactor/cleanup-naming

Conversation

@giunatale

@giunatale giunatale commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Pre-release cleanup in three logical commits: dead code out, names made
consistent. No behavior changes.

1. Remove the standalone-changeover path, keep the wire format reserved

The standalone-to-consumer changeover ("preVAAS") was never completed here and
its remnants were dead: the collections, the InitGenesis branch, the keeper
methods, and the standalone staking-keeper plumbing are removed. The two proto
fields stay reserved with comments documenting their intended future use,
so a later implementation can return without a wire-format conflict, and the
transition document is reframed as a forward-looking spec.

2. Remove fork-era dead code and stale comments

The dead IBC v1 port/channel cluster, unused channel-era sentinel errors and
key prefixes, an unused proto message, dead test utilities, hardcoded reserved
chain ids from the upstream project, stale upstream issue references, and the
accumulated stale comments describing code that no longer exists. Also adds a
tidy make target for the multi-module workspace.

3. Naming consistency

  • Provider module store name provider -> vaasprovider, matching
    vaasconsumer (pre-release: no migration concerns).
  • The CrossChainValidator family -> VaasValidator
    (ApplyCCValidatorChanges -> ApplyVaasValidatorChanges, accessors, proto
    message + regeneration) — the old name described the upstream project's
    architecture, not this one.
  • Rewriting the shared event/attribute constant block means a merge with any PR
    that adds a constant there takes this branch's version wholesale and drops
    theirs, so the sibling PRs' additions are defined here too, prefixed where the convention applies (AttributeKeyAckError, EventTypeConsumerEvidenceRejected, EventTypeRetireConsumer, AttributeConsumerClientID); each is harmless ahead of its user.
  • Three app/ files carried trailing whitespace inside comments, unformatted on
    main and invisible to make lint (golangci-lint's gofumpt does not flag it,
    gofmt -l does). Normalized.
  • Event types unified under a vaas_ prefix.
  • The operator guides' command examples swept to the renamed CLI route
    (query provider ... -> query vaasprovider ..., same for tx).
  • Package docs for the no_valupdates_* modules, and removal of inert
    plumbing that validated and passed a permanently-false flag.

Also in this PR

  • A real lint-configuration bug: the gci import-group list ended with a prefix
    (github.com/atomone-hub/atomone) that matches nothing in this repo, so the
    project's own imports fell into the catch-all group and their placement was
    never enforced. Corrected to github.com/allinbits/vaas, which is what
    AGENTS.md documents, and the resulting import re-sort is applied tree-wide.
    Four inert fork-era exclusion rules are dropped at the same time (two for an
    x/gov directory that does not exist, two targeting a linter that is not
    enabled, one for a migrations path that does not exist).
  • All 37 provider *KeyName constants are gone: 24 were unreferenced and the
    other 13 became inline names, matching the 25 collections that already used
    that form — the split was what let the dead two-thirds accumulate unnoticed.
  • Two entire files that only declared unused symbols, the fork-era
    IBCTransferKeeper expected keeper (with regenerated mocks), three app
    accessors that no interface requires, several dead e2e helpers, and the last
    CCV-era vocabulary — including three runtime strings an operator could
    actually see.
  • Stale comments describing code that no longer exists: a "CCV channel
    handshake" that IBC v2 has no concept of, a test name that was deleted long
    ago, three fictitious interfaces, and prefix names that had been renamed.

Testing

Build, lint, and the full unit suite green; the final tree was verified
byte-identical across the consolidation, and a trial-merge across all open
branches confirmed the rename reconciliation against their new code is a
handful of identifier fixups.

@giunatale

Copy link
Copy Markdown
Contributor Author

Branched from giunatale/feat/offline-detection (#63).
Opened against that branch so only this PR's commit shows; will retarget to main after #63 lands and this rebases.

Merge this one last from the series #63 to #75: it renames identifiers that other PRs' new code uses,
so landing it last concentrates the (small) rename reconciliation into its own final rebase instead of rippling through every open PR.

@giunatale
giunatale force-pushed the giunatale/refactor/cleanup-naming branch 2 times, most recently from 07a2e57 to 90d9305 Compare July 31, 2026 19:56

@julienrbrt julienrbrt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great cleanup

giunatale added a commit that referenced this pull request Aug 26, 2026
…k at the first wall-clock block

Review round for PR #66.

Photon-only fees deadlocked the chain the moment the policy activated:
every transaction needed a photon-voucher fee, including the relayer's
packet deliveries, which are the only way vouchers can ever arrive. The
ante now exempts transactions made up exclusively of infrastructure
messages (/ibc.core.* and /cosmos.gov.*, with the user-originating
MsgSendPacket carved out); what they pay is left to node-local
min-gas-prices, so a tokenless core shard launches with zero prices and
genesis-declared relayer and owner accounts.

The VSC staleness clock armed at NewChain InitGenesis with the genesis
block time, which is the genesis file's creation time and may predate
launch, burning safe-mode budget before the first block. It now arms in
BeginBlock at the first block after genesis, the first one carrying
wall-clock time; a restart genesis without the field arms the same way
instead of staying unarmed forever.

Docs are aligned with two facts the review surfaced: only vouchers from
the pinned client are accepted (deliberate: any other path is either
unverifiable or needs denom whitelisting), and the reference consumer
app wires no governance module, so consumer params are genesis-frozen
there and the photon flag cannot be flipped after launch. The liveness
query doc now shows the REST path that exists on this branch instead of
CLI syntax that lands with #74/#76.

The main e2e suite now runs its consumer with photon fees enabled from
genesis, and a new test proves the policy end to end: a native-fee tx
is rejected, an ICS-20 v2 transfer bridges uphoton in while enforcement
is on (the exemption at work), and a voucher-fee tx commits.
@giunatale
giunatale force-pushed the giunatale/refactor/cleanup-naming branch from 90d9305 to dba870b Compare August 26, 2026 12:24
@giunatale

Copy link
Copy Markdown
Contributor Author

Note for the final rebase (this PR merges last): two reconciliations cannot land earlier because their sites are not on this branch.

  1. The client-authentication chain's MsgSetProviderClient handler emits EventTypeChannelEstablished, a constant this branch deletes; rename that one emission to EventTypeClientEstablished at the rebase (same reconciliation already applied to relay.go here).
  2. Re-run the docs CLI-route sweep over doc lines merged after this branch's sweep commit, e.g. fix consumer CLI queries, localnet tooling, and consumer export #74's consumer-liveness command line.

The sibling branches' new event/attribute constants are already pre-defined here (d0d458f) so the constant-block conflicts resolve mechanically.

@giunatale

Copy link
Copy Markdown
Contributor Author

The retire event constant pre-defined earlier is dropped again: #78 folded MsgRetireConsumer into MsgRemoveConsumer per review there, so vaas_retire_consumer no longer has a user on any branch. The other pre-defined constants stand.

@giunatale
giunatale force-pushed the giunatale/refactor/cleanup-naming branch from 4cf33c4 to ed8a203 Compare August 26, 2026 15:23
giunatale added a commit that referenced this pull request Aug 27, 2026
…k at the first wall-clock block

Review round for PR #66.

Photon-only fees deadlocked the chain the moment the policy activated:
every transaction needed a photon-voucher fee, including the relayer's
packet deliveries, which are the only way vouchers can ever arrive. The
ante now exempts transactions made up exclusively of infrastructure
messages (/ibc.core.* and /cosmos.gov.*, with the user-originating
MsgSendPacket carved out); what they pay is left to node-local
min-gas-prices, so a tokenless core shard launches with zero prices and
genesis-declared relayer and owner accounts.

The VSC staleness clock armed at NewChain InitGenesis with the genesis
block time, which is the genesis file's creation time and may predate
launch, burning safe-mode budget before the first block. It now arms in
BeginBlock at the first block after genesis, the first one carrying
wall-clock time; a restart genesis without the field arms the same way
instead of staying unarmed forever.

Docs are aligned with two facts the review surfaced: only vouchers from
the pinned client are accepted (deliberate: any other path is either
unverifiable or needs denom whitelisting), and the reference consumer
app wires no governance module, so consumer params are genesis-frozen
there and the photon flag cannot be flipped after launch. The liveness
query doc now shows the REST path that exists on this branch instead of
CLI syntax that lands with #74/#76.

The main e2e suite now runs its consumer with photon fees enabled from
genesis, and a new test proves the policy end to end: a native-fee tx
is rejected, an ICS-20 v2 transfer bridges uphoton in while enforcement
is on (the exemption at work), and a voucher-fee tx commits.
…rved

Delete the dead standalone-to-consumer changeover Go (the PreVAAS/PrevStandaloneChain
state, standaloneStakingKeeper, the InitGenesis PreVAAS branch and its methods) and
rewrite docs/consumer-transition.md as a forward-looking reserved spec. The preVAAS
proto fields are kept, documented-reserved, for a future re-add.
Delete unused channel/handshake-era sentinels, the IBC-v1 port cluster, the Status
type and unused key prefixes, the KeyAssignmentReplacement proto message, dead testutil
crypto helpers, and the symbols orphaned by the PreVAAS removal (InitialValSet
collection + prefix, the PreVAAS/PrevStandaloneChain prefixes, an orphaned testutil
helper); reword stale CCV/channel/ICS-heritage comments; rename channel_established to
client_established; add a tidy make target.
Rename the provider module to vaasprovider (matches vaasconsumer), CrossChainValidator
to VaasValidator (proto + regen + accessors), and unify event-type strings under a
vaas_ prefix; add package docs to the no_valupdates wrapper modules. Flatten the
consumer genesis Validate to drop the now-dead preVAAS branch (keeping the load-bearing
provider-client checks) and remove the preVAAS constructor param / provider passing.
The module rename changes every provider cli route from 'provider' to
'vaasprovider'; the operator guides' command examples now say so too.
Same reasoning as AttributeKeyAckError: the naming commit rewrites the
event/attribute constant blocks wholesale, so a constant a sibling PR
adds there would be dropped out from under its user at the final
rebase. The rejected-evidence event lands with its vaas_ prefix this
way, and the retire event and declared-client attribute survive.
…umer

PR #78 now removes pre-launch consumers through MsgRemoveConsumer and
emits the remove event on both arms, so vaas_retire_consumer has no
user anywhere.
@giunatale
giunatale force-pushed the giunatale/refactor/cleanup-naming branch from ed8a203 to 45e4836 Compare August 27, 2026 13:59
@giunatale

Copy link
Copy Markdown
Contributor Author

Another dead constant to drop at the final rebase, same class as the retire one: #69's review removed the only user of AttributeKeyAckError (the evidence-rejection event no longer carries the ack bytes, since an IBC v2 error ack is a sentinel constant with no application error). Once #69 lands, AttributeKeyAckError has no user on any branch and its declaration in the shared events block should go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants