Skip to content

test: assert vesting-account creation is rejected via group proposal - #4635

Merged
kingpinXD merged 4 commits into
mainfrom
test/disallow-vesting-via-group-proposal
Aug 25, 2026
Merged

kingpinXD merged 4 commits into
mainfrom
test/disallow-vesting-via-group-proposal

Conversation

@kingpinXD

@kingpinXD kingpinXD commented Aug 25, 2026 •

Copy link
Copy Markdown
Member

Summary

We block vesting-account creation on ZetaChain, but the ante-level block (VestingAccountDecorator) only catches top-level txs. Governance-style wrappers (authz.MsgExec, group.MsgSubmitProposal) execute their inner messages through the msg router, which bypasses that decorator.

These tests pin the behavior that actually closes the group route: zeta's custom AuthzLimiterDecorator recurses into group.MsgSubmitProposal (and authz.MsgExec) inner messages and rejects any disabled type — including MsgCreateVestingAccount — at ante time, before the group module runs.

What we check:

  • Unit (app/ante/authz_test.go): AuthzLimiterDecorator blocks MsgCreateVestingAccount / MsgCreatePermanentLockedAccount when wrapped in a group proposal, an authz exec, and a nested authz→group combination; a non-disabled message (bank send) in the same group proposal passes; a top-level vesting msg is not blocked here (that's VestingAccountDecorator's job).
  • E2E (disallow_vesting_via_group_proposal, admin test group): broadcasting a group.MsgSubmitProposal that wraps MsgCreateVestingAccount fails with found disabled msg type. No real group needs to exist — the ante rejects the tx first.

🤖 Generated with Claude Code


Note

Low Risk
Test-only changes; no modifications to ante handlers or production behavior.

Overview
Adds regression coverage for the existing AuthzLimiterDecorator path that rejects disabled vesting-account creation messages when they are wrapped in group.MsgSubmitProposal or authz.MsgExec (including nested authz → group), without changing chain logic.

Unit tests in app/ante/authz_test.go table-drive ante handling: blocked cases expect found disabled msg type; allowed cases include a group proposal with a bank send and a top-level vesting msg (explicitly not this decorator’s responsibility).

E2E adds disallow_vesting_via_group_proposal: broadcasts a group proposal wrapping MsgCreateVestingAccount and asserts broadcast fails with the same error. The test is registered in the admin suite and local --test-admin run list.

Reviewed by Cursor Bugbot for commit 375afcc. Configure here.

Greptile Summary

The PR adds unit and end-to-end coverage confirming that disabled vesting-account creation messages are rejected when wrapped in group proposals or authz execution.

  • Adds table-driven ante tests for direct, wrapped, and nested message shapes.
  • Registers an admin E2E test that broadcasts a group proposal containing a vesting-account creation message.
  • Adds the new E2E test to the local admin suite.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking gap in coverage for the periodic vesting-account message.

The added unit and E2E paths correctly target recursive group and authz inspection, but the unit table does not exercise one of the three disabled vesting message types declared by its fixture.

Files Needing Attention: app/ante/authz_test.go

Important Files Changed

Filename Overview
app/ante/authz_test.go Adds focused recursive-wrapper tests, but omits a case for the periodic vesting message included in the disabled fixture.
e2e/e2etests/test_disallow_vesting_via_group_proposal.go Adds a live broadcast assertion that specifically requires the expected ante rejection text.
e2e/e2etests/e2etests.go Registers the new argument-free E2E test in the shared catalog.
cmd/zetae2e/local/local.go Adds the new test to the local admin suite with no identified orchestration issue.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Signed transaction] --> B[AuthzLimiterDecorator]
    B --> C{Wrapper type}
    C -->|group proposal| D[Inspect proposal messages]
    C -->|authz exec| E[Inspect executed messages]
    D --> F{Disabled vesting type?}
    E --> F
    F -->|Yes| G[Reject during ante handling]
    F -->|No| H[Continue ante chain]
Loading
Prompt To Fix All With AI
### Issue 1
app/ante/authz_test.go:30
**Periodic vesting case is untested**

The fixture includes `MsgCreatePeriodicVestingAccount` among the disabled types, but the table only wraps regular and permanent-locked vesting messages. Add a periodic-vesting case so this suite cannot pass without verifying the claimed rejection for all three disabled vesting-account creation types.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test: assert vesting-account creation is..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used (3)

Add a unit test for AuthzLimiterDecorator and an e2e test proving that
MsgCreateVestingAccount cannot be smuggled onto the chain by wrapping it
in a group proposal. The ante decorator inspects group.MsgSubmitProposal
inner messages, so the tx is rejected before the group module runs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9YpkFxXRRB1DES7ecggND
@kingpinXD
kingpinXD requested a review from a team as a code owner August 25, 2026 06:05
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9YpkFxXRRB1DES7ecggND
Comment thread app/ante/authz_test.go
Comment thread app/ante/authz_test.go
Comment thread app/ante/authz_test.go Outdated

@skosito skosito 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.

Approving — confirms group/authz-wrapped vesting creation is blocked at ante, including nested, with a negative case and a real e2e broadcast. Two non-blocking notes inline (gov path is out of scope; test hardcodes the disabled list rather than asserting app.go's wiring).

… + gov

- Export app.DisabledAuthzMsgs() as the single source of truth for the ante
  HandlerOptions; the unit test now asserts against it so dropping a vesting
  entry from app.go fails the test (no more hardcoded mirror).
- Add the MsgCreatePeriodicVestingAccount group-proposal case.
- Document the gov gap with a case asserting gov.MsgSubmitProposal is not
  covered by AuthzLimiterDecorator (privileged, out of scope).
- Add an in-process integration test running the real production ante handler
  end to end, proving the group-wrapped MsgCreateVestingAccount is rejected
  without needing localnet/Docker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9YpkFxXRRB1DES7ecggND

@morde08 morde08 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at dd63870, after the review-address commit. I checked the branch out, ran the suite, and mutation-tested each new test against the production code.

Verified working: TestAuthzLimiter_AnteHandle genuinely binds — I removed the recursion from the *group.MsgSubmitProposal case in app/ante/authz.go and 4 subtests went red. This is the first coverage AuthzLimiterDecorator has had since #794 (2023). The e2e mechanics also check out: AuthzLimiterDecorator is ante decorator #2, ahead of ValidateBasicDecorator, so the tx really does fail on found disabled msg type and not on the arbitrary group-policy address; BroadcastSync surfaces RawLog and errRetryable delegates Error(), so ErrorContains will match. The gov out-of-scope case and the periodic-vesting case both landed well.

Six things below. The first is the one I'd block on — everything else is hardening.

Comment thread cmd/zetae2e/local/local.go
Comment thread app/ante/authz_integration_test.go Outdated
Comment thread app/ante/authz_integration_test.go
Comment thread app/ante/authz_integration_test.go Outdated
Comment thread e2e/e2etests/test_disallow_vesting_via_group_proposal.go Outdated
Comment thread app/ante/authz_test.go
@kingpinXD kingpinXD added the ADMIN_TESTS Run make start-admin-tests label Aug 25, 2026
…roadcast

- extract AnteHandlerOptions so New and the integration test share one
  construction; dropping DisabledAuthzMsgs now fails the test
- //go:build test on authz_integration_test.go so a bare `go test ./app/ante/...`
  no longer panics
- add authz.MsgGrant coverage to the AuthzLimiter table
- BroadcastTxWithoutRetry for deterministic-failure e2e paths; use it in the
  disallow-vesting-via-group-proposal test to skip the 25s retry loop

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9YpkFxXRRB1DES7ecggND
@kingpinXD
kingpinXD requested a review from morde08 August 25, 2026 06:44
@kingpinXD
kingpinXD enabled auto-merge August 25, 2026 06:51
@kingpinXD
kingpinXD added this pull request to the merge queue Aug 25, 2026
Merged via the queue into main with commit 4ceb665 Aug 25, 2026
48 checks passed
@kingpinXD
kingpinXD deleted the test/disallow-vesting-via-group-proposal branch August 25, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ADMIN_TESTS Run make start-admin-tests breaking:cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants