Skip to content

fix: poa emit post-create validator tokens#139

Open
kpitapeersyst wants to merge 2 commits into
mainfrom
fix/poa-add-validator-event-tokens
Open

fix: poa emit post-create validator tokens#139
kpitapeersyst wants to merge 2 commits into
mainfrom
fix/poa-add-validator-event-tokens

Conversation

@kpitapeersyst
Copy link
Copy Markdown
Contributor

@kpitapeersyst kpitapeersyst commented May 4, 2026

fix: poa emit post-create validator tokens

Motivation 💡

  • ExecuteAddValidator emitted staking_tokens using the validator state fetched before MsgCreateValidator executed. That pre-create validator state has zero tokens, so the add_validator event did not reflect the validator created in staking state.

Changes 🛠

  • Re-fetch the validator after the MsgCreateValidator handler succeeds.
  • Emit AttributeStakingTokens from the post-create validator state.
  • Add focused keeper coverage for the add_validator event token value.

Summary by CodeRabbit

  • Bug Fixes

    • Validator creation events now report the correct staking token amount after the validator is created, ensuring transaction/event records reflect the post-creation stake.
  • Tests

    • Added a unit test that verifies the validator-add event emits the post-creation staking token value and validates this behavior in test coverage.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

📝 Walkthrough

Walkthrough

ExecuteAddValidator now re-queries the staking keeper for the validator after creation and emits EventTypeAddValidator with AttributeStakingTokens taken from the newly created validator's Tokens. Tests add tighter staking mocks and a new test asserting the post-create token value is emitted.

Changes

Post-Creation Token Emission

Layer / File(s) Summary
Data / Query
x/poa/keeper/keeper.go
After running the create-validator handler, the keeper calls k.sk.GetValidator again to obtain the created validator (newValidator).
Core Implementation
x/poa/keeper/keeper.go
Event emission for types.EventTypeAddValidator now uses newValidator.Tokens for types.AttributeStakingTokens instead of the earlier pre-create validator.Tokens.
Mocks / Test Helpers
x/poa/keeper/keeper_test.go
Added successfulAddValidatorStakingMocks(ctx, stakingKeeper) helper to centralize GetValidator expectations, including returns of sdk.DefaultPowerReduction. Added fmt import for assertions.
Tests / Verification
x/poa/keeper/keeper_test.go
Adjusted GetValidator expectations in existing cases (including the "validator not found when iterating" scenario). Added TestKeeper_ExecuteAddValidator_EmitsPostCreateStakingTokens to assert the emitted types.EventTypeAddValidator contains types.AttributeStakingTokens equal to fmt.Sprintf("%d", sdk.DefaultPowerReduction).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • AdriaCarrera

Poem

A rabbit hops and taps the keys,
Rechecks the garden after planting seeds.
Tokens counted once they're grown,
Events sing truths newly shown. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing the PoA module to emit post-creation validator token values in the add validator event.
Description check ✅ Passed The description follows the template structure with Motivation, Changes, but lacks Considerations and Dependencies sections. However, the required sections are complete with clear explanations of the problem and solution.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/poa-add-validator-event-tokens

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kpitapeersyst kpitapeersyst force-pushed the fix/poa-add-validator-event-tokens branch from 1b853c5 to 83a8653 Compare May 4, 2026 18:05
Copy link
Copy Markdown
Contributor

@AdriaCarrera AdriaCarrera left a comment

Choose a reason for hiding this comment

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

It's better to remove the event attribute itself

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
x/poa/keeper/keeper_test.go (1)

418-423: ⚡ Quick win

Avoid asserting on the last event index directly.

Using events[len(events)-1] makes this test fragile if any later event is added. Search for types.EventTypeAddValidator in the emitted events and assert on that match instead.

Suggested test hardening
-	event := events[len(events)-1]
-	require.Equal(t, types.EventTypeAddValidator, event.Type)
-
-	attribute, ok := event.GetAttribute(types.AttributeStakingTokens)
+	var event sdk.Event
+	found := false
+	for i := len(events) - 1; i >= 0; i-- {
+		if events[i].Type == types.EventTypeAddValidator {
+			event = events[i]
+			found = true
+			break
+		}
+	}
+	require.True(t, found, "add_validator event not found")
+
+	attribute, ok := event.GetAttribute(types.AttributeStakingTokens)
 	require.True(t, ok)
 	require.Equal(t, fmt.Sprintf("%d", sdk.DefaultPowerReduction), attribute.Value)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@x/poa/keeper/keeper_test.go` around lines 418 - 423, The test currently
assumes the EventTypeAddValidator is the last emitted event by using
events[len(events)-1]; instead, iterate or search the events slice for an event
with Type == types.EventTypeAddValidator (e.g., loop over events or use a helper
to find the first matching event), assert that such an event exists, then call
event.GetAttribute(types.AttributeStakingTokens) and check the attribute.Value
equals fmt.Sprintf("%d", sdk.DefaultPowerReduction); update the assertions in
keeper_test.go to reference the found event rather than the last index to avoid
fragility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@x/poa/keeper/keeper_test.go`:
- Around line 418-423: The test currently assumes the EventTypeAddValidator is
the last emitted event by using events[len(events)-1]; instead, iterate or
search the events slice for an event with Type == types.EventTypeAddValidator
(e.g., loop over events or use a helper to find the first matching event),
assert that such an event exists, then call
event.GetAttribute(types.AttributeStakingTokens) and check the attribute.Value
equals fmt.Sprintf("%d", sdk.DefaultPowerReduction); update the assertions in
keeper_test.go to reference the found event rather than the last index to avoid
fragility.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5655b91c-1fb1-4f71-ac78-f3a5ddb7fccd

📥 Commits

Reviewing files that changed from the base of the PR and between 83a8653 and 74715b0.

📒 Files selected for processing (1)
  • x/poa/keeper/keeper_test.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