Skip to content

Conversation

@mateeullahmalik
Copy link
Collaborator

@mateeullahmalik mateeullahmalik commented Nov 16, 2025

Description

This PR removes the redundant LocalCosmosAddress field from the SDK’s AccountConfig and rewires the SDK to derive the signer’s Cosmos address internally from the provided Keyring + KeyName.

This change improves correctness, reduces configuration surface area, and ensures SDK behavior cannot drift due to mismatched config inputs.


Why This Change Was Needed

❌ Previous Design Problems

  • Redundant configuration: callers had to supply both KeyName and the corresponding LocalCosmosAddress.
  • Risk of divergence: if the provided address didn’t match the keyring’s actual key record, signature verification failures could occur in unpredictable ways.
  • Leaky abstraction: address resolution (a deterministic process) was incorrectly pushed onto SDK users rather than handled internally.

✔️ New Design Principles

  • SDK config should contain only primary inputs, not derived data.

  • The Cosmos address is fully determined by (Keyring, KeyName).

  • The SDK should internally and reliably perform:

    rec := keyring.Key(KeyName)
    addr := rec.GetAddress()
  • This makes the SDK simpler, safer, and impossible to misconfigure.


What Was Changed

1. Removed LocalCosmosAddress from AccountConfig

type AccountConfig struct {
    KeyName  string
    Keyring  keyring.Keyring
    PeerType PeerType  // unchanged
}

Callers now only provide:

AccountConfig{
    KeyName: <key>,
    Keyring: <keyring>,
}

2. Updated FactoryConfig and ClientFactory

  • Removed LocalCosmosAddress from FactoryConfig.
  • Added KeyName instead.
  • ClientFactory now resolves and caches the signer’s address internally:
addr, err := keyringpkg.GetAddress(kr, cfg.KeyName)
f.address = addr.String()
  • All supernode request signatures and auth flows now use the internally-derived address.

3. Updated CascadeTask

Replaced:

LocalCosmosAddress: t.config.Account.LocalCosmosAddress

with:

KeyName: t.config.Account.KeyName

Since address resolution now happens inside the factory, the task no longer needs an explicit address.


4. Updated all internal call-sites

Any SDK module that previously referenced:

cfg.Account.LocalCosmosAddress

now uses:

  • client.signerAddr (cached once in constructor), or
  • keyringpkg.GetAddress(cfg.Account.Keyring, cfg.Account.KeyName) if needed temporarily.

Backward Compatibility

✔️ No breaking changes for chains, Lumera modules, or Supernode protocols.
✔️ No changes to on-chain metadata format.
✔️ No changes to StartCascade / Download flows.

Breaking change for SDK callers:
They must remove LocalCosmosAddress from their config and only pass KeyName + Keyring.

(This change is deliberate and improves correctness.)


Benefits

  • Eliminates entire class of misconfiguration bugs.
  • Guarantees consistency between keyring keys and signer address.
  • Reduces API surface and cognitive load for SDK users.
  • Moves deterministic logic into the SDK where it belongs.
  • Prepares the SDK for future improvements (ADR-36 meta-signing, multi-address accounts, etc.).

Testing

  • Updated integration tests to stop passing LocalCosmosAddress.
  • Verified Cascade upload → registration → tx hash → finalize → download flows.
  • Verified all internal auth signatures use the keyring-derived address.
  • Confirmed compatibility with existing on-chain MsgRequestAction validations.

@mateeullahmalik mateeullahmalik changed the title fix cascade e2e tests Remove LocalCosmosAddress from SDK Config + Derive Address Internally From Keyring Nov 16, 2025
@mateeullahmalik mateeullahmalik merged commit fb7b737 into master Nov 16, 2025
7 checks passed
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