Skip to content

Conversation

@akobrin1
Copy link
Contributor

@akobrin1 akobrin1 commented Jan 7, 2026

  • Adds support for verifying signatures on actions submitted via ICA (app_pubkey), with a fallback to on-chain verification.
  • Refactors cascade signature verification logging to reuse base log fields via logtrace.WithFields.
  • Lowers P2P worker shutdown logs from ERROR to INFO to reduce noise.
  • Updates tests and gomock usage to match external lumera mocks; fixes keyring signing call signature.

@roomote
Copy link

roomote bot commented Jan 7, 2026

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The PR implements ICA-based signature verification for cascade actions with proper fallback to on-chain verification. The changes include:

  • New ICA signature verification logic in SDK and supernode cascade modules
  • Refactored logging to use logtrace.WithFields for cleaner field merging
  • P2P worker shutdown logs changed from ERROR to INFO (appropriate for graceful shutdown)
  • New AccountByAddress and QueryTxsByEvents methods in auth/tx modules
  • Good test coverage for the new ICA verification paths

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

roomote[bot]
roomote bot previously approved these changes Jan 7, 2026
Copy link
Contributor

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 adds support for verifying signatures on actions submitted via ICA (Interchain Accounts) using an app_pubkey field, with fallback to on-chain verification. The implementation includes comprehensive refactoring of cascade signature verification, test infrastructure improvements, and dependency updates.

Key changes:

  • Adds ICA-based signature verification using app_pubkey with fallback to on-chain verification
  • Refactors cascade signature verification to use reusable base log fields via logtrace.WithFields
  • Updates test infrastructure to disable state sync and improve supernode logging
  • Migrates from github.com/golang/mock to go.uber.org/mock

Reviewed changes

Copilot reviewed 56 out of 61 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/tools.go Adds build tag for gomock tool dependency management
tests/system/system.go Adds state sync disabling in test configs
tests/system/supernode-utils.go Adds supernode log file capture to disk
tests/system/go.mod/go.sum Updates Go version, dependencies, and adds ICA test support
tests/system/e2e_cascade_test.go Adds comprehensive ICA E2E test flow with signature verification
tests/integration/securegrpc/secure_connection_test.go Updates gomock import path
supernode/cascade/register.go Passes app_pubkey to signature validation
supernode/cascade/ica_verify.go New file implementing ICA signature verification logic
supernode/cascade/ica_verify_test.go New tests for ICA signature verification
supernode/cascade/helper.go Refactors to use new signature verifier builder
supernode/cascade/download.go Adds ICA signature verification for downloads with improved logging
sdk/task/manager.go Adds PublishEvent method
sdk/task/task.go Improves supernode rejection logging
sdk/task/helpers.go Adds ICA signature validation with fallback logic
sdk/task/cascade.go Adds logging for zero eligible supernodes
sdk/event/types.go Adds SDKMetadataICASigned event type
sn-manager/go.mod Updates Go version and dependencies

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

@akobrin1 akobrin1 requested a review from a-ok123 January 7, 2026 19:27
@akobrin1 akobrin1 merged commit 8b100f9 into master Jan 8, 2026
13 checks passed
@akobrin1 akobrin1 deleted the ica-support branch January 8, 2026 23:25
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