Skip to content

feat: Add AggregationISM to hyperlane-cosmos#152

Open
jonas089 wants to merge 5 commits intobcp-innovations:mainfrom
jonas089:jonas/aggregation
Open

feat: Add AggregationISM to hyperlane-cosmos#152
jonas089 wants to merge 5 commits intobcp-innovations:mainfrom
jonas089:jonas/aggregation

Conversation

@jonas089
Copy link
Copy Markdown
Collaborator

@jonas089 jonas089 commented Jan 21, 2026

This PR introduces a full implementation of AggregationISM for the hyperlane-cosmos module.

AggregationISM allows composing multiple existing ISMs and enforcing an M-of-N verification policy. Message verification succeeds only if at least threshold child ISMs successfully verify the message.

New ISM type

  • AggregationISM with on-chain storage, validation, and routing
  • Supports nested aggregation (aggregation ISMs can reference other aggregation ISMs)

Verification logic

  • Calls Verify() on each configured child ISM via the core keeper
  • Early-exit once the threshold is met (gas-efficient)
  • Returns detailed errors when verification fails

State & ownership management

  • Owner-controlled updates for:
    • Child module list
    • Threshold
    • Ownership transfer or permanent renouncement
  • Ownership checks enforced at the keeper level

Messages & events

  • MsgCreateAggregationIsm
  • MsgSetAggregationIsmModules
  • MsgUpdateAggregationIsmOwner
  • Corresponding typed events for all state changes

Validation

  • Threshold > 0 and ≤ number of modules
  • No duplicate modules
  • All referenced ISMs must exist
  • Strict ownership and address validation

Tests

  • Comprehensive unit and integration tests covering:
    • Success and failure paths
    • Edge cases (invalid configs, missing ISMs)
    • Nested aggregation
    • Gas behavior with large module sets

@mbreithecker
Copy link
Copy Markdown
Member

Thanks for your PR.

Why are you removing all the "Arrange", "Act" and "Assert" comments from the test files?

Can you please make sure that CI passes. You can also run make all locally.

@jonas089
Copy link
Copy Markdown
Collaborator Author

jonas089 commented Feb 4, 2026

@mbreithecker I re-added the comments. Removed them because I thought they were not very useful, but I understand that you want to keep them.

Working on CI now.

@jonas089 jonas089 changed the title Feat: Add AggregationISM to hyperlane-cosmos feat: Add AggregationISM to hyperlane-cosmos Feb 4, 2026
@mbreithecker mbreithecker requested a review from Copilot February 5, 2026 10:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements AggregationISM for the hyperlane-cosmos module, enabling M-of-N verification by composing multiple existing ISMs. The implementation validates messages when a configurable threshold of child ISMs successfully verify them.

Changes:

  • Added AggregationISM type with threshold-based verification and ownership management
  • Implemented keeper methods for creating and updating aggregation ISMs
  • Added comprehensive validation and testing for edge cases and nested aggregation

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
x/core/01_interchain_security/types/types.pb.go Generated protobuf code for AggregationISM type definition
x/core/01_interchain_security/types/msg_aggregation.go Message validation for aggregation ISM operations
x/core/01_interchain_security/types/events.pb.go Generated event definitions for aggregation ISM lifecycle
x/core/01_interchain_security/types/errors.go New error types for aggregation validation
x/core/01_interchain_security/types/codec.go Interface registration for aggregation messages and types
x/core/01_interchain_security/types/aggregation_ism_test.go Unit tests for validation logic
x/core/01_interchain_security/types/aggregation_ism.go Core aggregation ISM implementation with validation
x/core/01_interchain_security/keeper/msg_server_test.go Integration tests for message handlers
x/core/01_interchain_security/keeper/msg_server.go Keeper methods for aggregation ISM CRUD operations
x/core/01_interchain_security/keeper/keeper.go Handler registration for aggregation ISM
x/core/01_interchain_security/keeper/genesis.go Genesis state handling for aggregation ISMs
x/core/01_interchain_security/keeper/aggregation_ism_handler_test.go Verification logic tests including nested aggregation
x/core/01_interchain_security/keeper/aggregation_ism_handler.go Aggregation verification handler with early exit optimization
tests/integration/mock.go Enhanced mock ISM to support pass/fail states
proto/hyperlane/core/interchain_security/v1/types.proto Protobuf schema for AggregationISM
proto/hyperlane/core/interchain_security/v1/tx.proto Protobuf definitions for aggregation messages
proto/hyperlane/core/interchain_security/v1/events.proto Event schemas for aggregation ISM operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 4 to 13
"bytes"
"context"

sdk "github.com/cosmos/cosmos-sdk/types"

"cosmossdk.io/errors"

"cosmossdk.io/collections"

"github.com/bcp-innovations/hyperlane-cosmos/util"
"cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/crypto"

"github.com/bcp-innovations/hyperlane-cosmos/util"
"github.com/bcp-innovations/hyperlane-cosmos/x/core/01_interchain_security/types"
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The 'bytes' and 'crypto' imports at lines 4 and 10 may no longer be necessary. These imports were likely used in the original code before the refactoring that extracted validateOwnershipTransfer and validateModulesExist. Consider removing unused imports to improve code maintainability.

Copilot uses AI. Check for mistakes.
Comment thread x/core/01_interchain_security/keeper/aggregation_ism_handler.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.

3 participants