Skip to content

fix(cosmwasm): coordinator migration defaults to multisig prover/chain map #1066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

fix(cosmwasm): coordinator migration defaults to multisig prover/chain map #1066

wants to merge 10 commits into from

Conversation

SDavidson1177
Copy link
Contributor

@SDavidson1177 SDavidson1177 commented Oct 2, 2025 •
edited by cursor bot
Loading

Description

The multisig contract maintains a chain->prover map, and the coordinator maintains a separate prover->chain map. In order to keep these maps consistent after migration, the migration script will default to using the multsig's map if they differ.

How is This Done

The script now queries the multisig contract first when constructing the coordinator's migration message. During migration, the coordinator will default to using the prover/chain pair found in the multisig. This script assumes multisig version 2.3 has been deployed.

Reference

Corresponding change in Amplifier:
axelarnetwork/axelar-amplifier#1058

Testing

This has been tested both on a custom devnet, and on devnet-amplifier. The results of running this migration on devnet-amplifier have been saved to devnet-amplifier.json.

Note

Adds a multisig migration path (to v2.3.1) and upgrades the coordinator migration to sync prover mappings from multisig, enforce uniqueness, support ignoreChains/direct modes, and update docs/config accordingly.

  • CosmWasm migration tooling:
    • Coordinator migration: refactors to use multisig as source for prover->chain, enforces uniqueness (provers/verifiers/gateways), supports ignoreChains, --direct, and fee param; updates titles to v2.1.1 and logs; camelCases types/vars.
    • New Multisig migration: adds multisig.ts with migration to v2.3.1; extracts default provers from config; supports direct/proposal execution and fee param.
    • CLI: migrate.ts adds ignoreChains and --direct; wires fee through; routes to coordinator or multisig migrators.
    • Types: replaces proposal with direct; adds ignoreChains.
  • Config:
    • axelar-chains-config/info/devnet-amplifier.json: bumps Router/Multisig/Coordinator codeIds and proposal ids/hashes (reflecting new deployments).
  • Docs:
    • releases/cosmwasm/2025-09-Coordinator-v2.1.0.md: marks devnet as completed, updates multisig to v2.3.1, revises migration steps to use new scripts/flags and notes ignoreChains caution.

Written by Cursor Bugbot for commit ca73407. This will update automatically on new commits. Configure here.

Greptile Overview

Updated On: 2025-10-02 17:29:09 UTC

Summary

This PR addresses a critical data consistency issue between the multisig and coordinator contracts in the CosmWasm migration system. The multisig contract maintains a `chain->prover` mapping while the coordinator maintains a separate `prover->chain` mapping, which could become inconsistent during migrations.

The key change is refactoring the coordinator migration script to query the multisig contract first and use its mapping as the authoritative source when constructing the coordinator's migration message. This ensures both contracts will have consistent mappings after migration completes.

The implementation adds several safety mechanisms:

  • A new ignoreChains parameter to exclude problematic chains during migration
  • Duplicate detection logic to validate uniqueness constraints for provers, verifiers, and gateways
  • Improved error handling with try-catch blocks to prevent single chain failures from blocking entire migrations
  • Better robustness by combining gateway queries with multisig prover lookups in a single operation

The changes also include code cleanup (removing unused imports, renaming unused parameters) and documentation updates to reflect the new migration strategy. The devnet-amplifier configuration shows successful execution of this migration approach, updating contract code IDs for coordinator v2.1.0, multisig v2.3.1, and router v1.3.0.

Important Files Changed

Changed Files
Filename Score Overview
cosmwasm/migrate/coordinator.ts 4/5 Refactored coordinator migration to query multisig contract for authoritative chain-to-prover mapping and added duplicate detection logic
cosmwasm/migrate/types.ts 5/5 Added optional ignoreChains parameter to MigrationOptions interface for excluding chains during migration
cosmwasm/migrate/migrate.ts 4/5 Cleaned up unused imports, renamed unused parameter, and added ignoreChains option support to migration orchestrator
releases/cosmwasm/2025-09-Coordinator-v2.1.0.md 4/5 Updated documentation to reflect migration strategy changes, multisig version bump to v2.3.1, and completed devnet-amplifier deployment
axelar-chains-config/info/devnet-amplifier.json 4/5 Updated contract code IDs and proposal IDs showing successful coordinator migration execution on devnet-amplifier environment

Confidence score: 4/5

  • This PR addresses a well-defined consistency problem with a logical solution that uses the multisig contract as the source of truth
  • Score reflects good architectural approach and thorough testing, but complexity of migration logic requires careful review
  • Pay close attention to the coordinator migration script and the empty "undefined" contract entry in devnet-amplifier.json that needs cleanup

Sequence Diagram

sequenceDiagram
    participant User
    participant Script as Migration Script
    participant Client as CosmWasm Client
    participant Router as Router Contract
    participant Multisig as Multisig Contract
    participant Coordinator as Coordinator Contract
    participant Governance as Governance System

    User->>Script: "Execute coordinator migration"
    Script->>Client: "Connect to CosmWasm client"
    
    Note over Script: Fetch chain configuration data
    Script->>Router: "Query chains from router"
    Router-->>Script: "Return chain endpoints"
    
    Note over Script: For each chain endpoint
    loop Process Chain Endpoints
        Script->>Client: "Query gateway config from chain"
        Client-->>Script: "Return verifier address"
        Script->>Multisig: "Query authorized prover for chain"
        Multisig-->>Script: "Return prover address"
        Script->>Script: "Build chain contract mapping"
    end
    
    Script->>Script: "Check for duplicate addresses"
    Note over Script: Validate uniqueness of provers, verifiers, gateways
    
    Script->>Script: "Construct migration message"
    Note over Script: Include router, multisig, and chain contracts
    
    alt Governance Proposal Mode
        Script->>Governance: "Submit migration proposal"
        Governance-->>Script: "Proposal submitted"
    else Direct Migration
        Script->>Client: "Execute direct migration"
        Client->>Coordinator: "Migrate to new code ID with message"
        Coordinator-->>Client: "Migration completed"
        Client-->>Script: "Migration successful"
    end
    
    Script-->>User: "Migration completed"
Loading

@SDavidson1177 SDavidson1177 changed the title coordinator migration defaults to multisig prover/chain map fix(cosmwasm): coordinator migration defaults to multisig prover/chain map Oct 2, 2025
@SDavidson1177 SDavidson1177 marked this pull request as ready for review October 2, 2025 17:27
@SDavidson1177 SDavidson1177 requested a review from a team as a code owner October 2, 2025 17:27
greptile-apps[bot]
greptile-apps bot reviewed Oct 2, 2025
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

cursor[bot]

This comment was marked as outdated.

Sign in to view

kulikthebird
kulikthebird requested changes Oct 6, 2025
View reviewed changes
Copy link
Contributor

@kulikthebird kulikthebird left a comment

Choose a reason for hiding this comment

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

Important change, thank you! Left some comments

cosmwasm/migrate/coordinator.ts Outdated

const chain_contracts: ChainContracts[] = [];

for (let i = 0; i < chain_endpoints.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use iterator

cosmwasm/migrate/coordinator.ts Outdated
interface GatewayConfig {
verifier: string;
}
function check_for_duplicates(chains: ChainContracts[]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

camelCase

cosmwasm/migrate/coordinator.ts Outdated
console.log('Executing migration...', migrate_options);
if (options.proposal) {
await submitProposal(client, config, migrate_options, proposal);
await submitProposal(client, config, migrate_options, proposal, options.fees);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the fee from migrate method argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'll use that

cosmwasm/migrate/coordinator.ts Outdated
await submitProposal(client, config, migrate_options, proposal, options.fees);
console.log('Migration proposal successfully submitted');
} else {
await client.migrate(sender_address, coordinator_address, Number(code_id), migration_msg, options.fees);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

cosmwasm/migrate/coordinator.ts Outdated
try {
const chain_contracts: ChainContracts[] = [];
chains.forEach((c) => {
if (c.prover_address && !provers.has(c.prover_address)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

camelCase

Copy link
Contributor

Choose a reason for hiding this comment

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

Here and in other places in this PR

releases/cosmwasm/2025-09-Coordinator-v2.1.0.md Outdated
Comment on lines 119 to 125
--msg "{\"coordinator\": \"$COORDINATOR_ADDRESS\", \"default_authorized_provers\": {\"chain1\": \"prover1\", \"chain2\":\"prover2\",...}}" \
--fetchCodeId \
--deposit $DEPOSIT_VALUE
```

The `default_authorized_provers` object should correspond to the chain/prover pairs located at `axelar.contracts.MultisigProver` in `$ENV.json`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate on this? Why the default_authorized_provers is not read automatically from the config if it's needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also it's important to take XRPL, Solana, Sui and Stellar Provers into account.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point. I'll modify it so that it is read from the config. I'll make sure it takes into account XRPL, Solana, Sui and Stellar

cursor[bot]

This comment was marked as outdated.

Sign in to view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulikthebird kulikthebird kulikthebird requested changes

@cursor cursor[bot] cursor[bot] left review comments

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SDavidson1177 @kulikthebird