refactor(poa)!: parse validator address as AccAddress

[aluque-peersyst]

parse validator address as AccAddress

The address parameter in MsgAddValidator and MsgRemoveValidator should always be of type AccAddress. ExecuteRemoveValidator parsed it as a ValAddress (ethmvaloper1…), contradicting the cosmos.AddressString proto annotation and AddValidator's behavior.

Changes

• Add a shared parseValidatorAddress helper (parses AccAddress, derives ValAddress)
• Use it in both ExecuteAddValidator and ExecuteRemoveValidator
• Swap RemoveValidator test fixtures from ethmvaloper1… to ethm1…
• Add TestParseValidatorAddress

Considerations

• BREAKING: MsgRemoveValidator now expects the account address (ethm1…), not the operator address (ethmvaloper1…).

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Corrected remove-validator address handling to consistently accept and process account-encoded addresses rather than validator-operator format.
  • Refined execution flow so address parsing happens before deriving dependent address values, improving correctness during unbonding.
• New Features
  • Added stateless validation for add-validator and remove-validator messages, returning clear errors for invalid addresses or missing/incorrect pubkeys.
• Tests
  • Expanded unit and integration coverage to match the updated address format expectations and validation behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 202fd434-bbc2-4782-ba48-4c212f2cf0eb

📥 Commits

Reviewing files that changed from the base of the PR and between d4546d3 and 0ee1fa4.

📒 Files selected for processing (7)

• tests/integration/poa_test.go
• x/poa/keeper/keeper.go
• x/poa/keeper/keeper_test.go
• x/poa/types/message_add_validator.go
• x/poa/types/message_add_validator_test.go
• x/poa/types/message_remove_validator.go
• x/poa/types/message_remove_validator_test.go

💤 Files with no reviewable changes (1)

• x/poa/keeper/keeper_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/integration/poa_test.go

---

📝 Walkthrough

Walkthrough

Introduces ValidateBasic() methods on MsgAddValidator and MsgRemoveValidator to validate authority and validator addresses as bech32 account addresses, with MsgAddValidator also validating pubkey presence and type. Updates ExecuteAddValidator and ExecuteRemoveValidator in the keeper to parse and validate addresses before deriving sdk.ValAddress. Updates all unit and integration test fixtures to use account-encoded addresses instead of validator-operator-encoded addresses.

Changes

POA message validation and keeper parsing fix

Layer / File(s)	Summary
Message type ValidateBasic implementations x/poa/types/message_add_validator.go, x/poa/types/message_remove_validator.go	MsgAddValidator and MsgRemoveValidator implement ValidateBasic() to parse Authority and ValidatorAddress as bech32 account addresses, returning ErrInvalidAddress on parse failure. MsgAddValidator additionally validates Pubkey is non-nil and contains a cryptotypes.PubKey type.
Message ValidateBasic unit tests x/poa/types/message_add_validator_test.go, x/poa/types/message_remove_validator_test.go	Adds table-driven tests TestMsgAddValidator_ValidateBasic and TestMsgRemoveValidator_ValidateBasic covering valid authority and validator addresses, invalid address formats, nil pubkey, and wrong pubkey message types with expected error substrings.
Keeper parsing order fix x/poa/keeper/keeper.go	ExecuteAddValidator and ExecuteRemoveValidator now parse and validate the bech32 address before deriving sdk.ValAddress. ExecuteRemoveValidator reuses the parsed accAddress when calling Unbond instead of converting from valAddress.
Test fixture updates: account address format x/poa/keeper/keeper_test.go, x/poa/keeper/msg_server_remove_validator_test.go, tests/integration/poa_test.go	Updates TestKeeper_ExecuteRemoveValidator table entries and TestMsgServer_RemoveValidator subtests to use account-bech32 addresses. Updates four integration test functions (TestRemoveValidator_UnexistingValidator, TestRemoveValidator_ExistingValidator_StatusBonded, TestRemoveValidator_ExistingValidator_Jailed, TestRemoveValidator_ExistingValidator_Tombstoned) to pass account-encoded addresses to NewMsgRemoveValidator. Updates gomock import to go.uber.org/mock/gomock.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• xrplevm/node#142: Modifies the same TestMsgServer_RemoveValidator test cases and ValidatorAddress values.

Suggested reviewers

• AdriaCarrera

Poem

🐰 Validation hops through addresses with care,
Account-encoded addresses everywhere!
The keeper now parses before taking the leap,
ValidateBasic checks run, validation runs deep.
From bech32 strings to cryptotype keys,
The format's aligned—the PR's sure to please! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% 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 'refactor(poa)!: parse validator address as AccAddress' accurately summarizes the main change—standardizing validator address parsing to use AccAddress instead of ValAddress across the POA module.
Description check	✅ Passed	The description covers the key aspects: motivation for the change (addressing contradiction with proto annotation), main changes made (shared helper function, test updates), and explicitly notes the breaking change requiring account addresses instead of operator addresses.
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 unit tests (beta)

• Create PR with unit tests

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[AdriaCarrera]

Also add ValidateBasic function for message validation

[AdriaCarrera]

This function is not needed. Remove it and perform the parsing at each time.

[AdriaCarrera]

Why not sdk.AccAddress(randomValAddr).String()?