Skip to content

fix(evmigration): keyring-backend auto-resolution and os-backend prompt fix for migration scripts#193

Merged
akobrin1 merged 11 commits into
masterfrom
fix/evmigration-os-keyring-prompt
Jul 17, 2026
Merged

fix(evmigration): keyring-backend auto-resolution and os-backend prompt fix for migration scripts#193
akobrin1 merged 11 commits into
masterfrom
fix/evmigration-os-keyring-prompt

Conversation

@akobrin1

Copy link
Copy Markdown
Contributor

Summary

Two related fixes to the EVM-migration shell tooling, found while debugging a live validator migration that appeared to hang:

1. os keyring backend hung with a hidden passphrase prompt (aa453270)
The passphrase-prompt stderr-tee in lumerad_tx/preview_tx_body/resolve_address was gated on KEYRING_BACKEND == "file". But cosmos-sdk's os backend does not pin AllowedBackends, so on a headless host it falls back to the encrypted file store and prompts identically — the prompt landed in a captured stderr file and the script sat silently waiting on stdin. New guard _keyring_prompts_for_passphrase (!= "test") covers any prompting backend.

2. Scripts no longer force --keyring-backend test by default (2f680412..3b0009a7)
When --keyring-backend is not passed, resolve_keyring_backend now resolves it the way lumerad users expect, first hit wins:

  1. explicit --keyring-backend flag
  2. keyring-backend from <home>/config/client.toml (--home selects home; --keyring-dir does not move client.toml)
  3. on-disk detection under --keyring-dir (else home): keyring-test/test, keyring-file/file
  4. os — the SDK default

The chosen value and its source are logged before anything signs. Wired into migrate-account.sh, migrate-validator.sh, and all three migrate-multisig.sh subcommands (resolved value propagates to child invocations as an explicit flag). Existing devnet/CI flows keep working via keyring-test/ disk detection.

Design + plan: docs/design/2026-07-16-keyring-backend-resolution-{design,plan}.md.

Also fixed along the way (5001452d):

  • resolve_keyring_backend crashed under set -u when $HOME is unset (systemd/cron runs) instead of reaching the os fallback.
  • Pre-existing test-infra bug: the lib's source-time EXIT trap clobbered bats' failure reporting in common.bats — failing tests died silently with no not ok diagnostics. setup() now saves/restores bats' trap.

Housekeeping (c3557ec6, 9f30f048): devnet-module x/crypto 0.52.0 bump (counterpart of #190, required for the module to resolve) and a validator-migration guide expansion on why stopping the node before broadcast is mandatory (equivocation/tombstone risk).

Test plan

  • bats tests/scripts/ — 193/193 (new coverage: resolver order/precedence/detection/fallback, unset-$HOME regression, os-prompt tee regression, client.toml e2e for account + multisig submit; gas-helper tests ported from the deleted stale scripts/evmigration-common_test.sh)
  • make lint — 0 issues
  • Manual: migrate-account.sh --dry-run against the shim with empty --home logs keyring backend: os (SDK default; ...)

🤖 Generated with Claude Code

akobrin1 and others added 10 commits July 16, 2026 14:45
Design spec for resolving the migration scripts' keyring backend from
--keyring-backend > client.toml > on-disk detection > os, instead of
hardcoding the `test` default (which disagrees with bare lumerad).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task-by-task TDD plan for resolve_keyring_backend, entry-script wiring,
multisig integration, and bats test consolidation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The migration wrappers teed the sign/preview stderr to the terminal only
when KEYRING_BACKEND == "file". The os backend falls back to the encrypted
file store on a headless host and prompts identically, but took the else
branch that redirected stderr to a temp file — hiding the "Enter keyring
passphrase" prompt so the command appeared to hang while waiting on stdin.

Introduce _keyring_prompts_for_passphrase (!= "test") and gate the tee on
it in lumerad_tx, preview_tx_body, and resolve_address.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t file

Port the gas-helper and _keyring_prompts_for_passphrase coverage from the
unwired scripts/evmigration-common_test.sh into the canonical
tests/scripts/common.bats suite (run by make via bats tests/scripts/),
and delete the stale file.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ault os

Add resolve_keyring_backend to the migration script lib: when no
--keyring-backend is passed, honor <home>/config/client.toml, then detect
keyring-test//keyring-file/ under --keyring-dir (else --home), then fall
back to the SDK default `os` instead of hardcoding `test`. Track explicit
flag usage via KEYRING_BACKEND_EXPLICIT in parse_common_flags.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…scripts

Call resolve_keyring_backend after parse_common_flags in both entry
scripts, and generalize the "keyring may prompt" advisory from the
file-only guard to _keyring_prompts_for_passphrase so os is covered too.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ubcommands

Track explicit --keyring-backend in each of the three subcommand parsers
(generate/sign/submit) and run resolve_keyring_backend after the globals
are wired, reading the resolved value back into the local so sub-invoked
lumerad calls receive it explicitly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… output

Review findings from the keyring-resolution branch:

- resolve_keyring_backend crashed under set -u when $HOME is unset
  (systemd/cron/env -i runs) instead of reaching the os fallback; guard
  with ${HOME:-}.

- common.bats setup() sources evmigration-common.sh, whose source-time
  cleanup EXIT trap replaced bats' own EXIT trap — the one that prints
  "not ok" diagnostics — so every failing test in the file died silently
  as "Executed N-1 instead of N". Save and restore bats' trap around the
  source so failures report properly again.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Devnet-module counterpart of #190 (root module bump); without it the
devnet module no longer resolves (go mod tidy error).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mandatory

Expand the validator-migration guide: the consensus key is unchanged by
migration, so a supervisor-respawned instance overlapping a manual restart
can double-sign and tombstone the validator (irreversible), vs the merely
recoverable cost of missed blocks when stopping cleanly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9f30f04895

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/evmigration-common.sh
Comment thread scripts/evmigration-common.sh
…default

PR #193 review follow-ups (Codex):

- resolve_keyring_backend now honors $LUMERA_KEYRING_BACKEND between the
  explicit flag and client.toml — the same env override lumerad itself
  honors (used by evmtest/devnet tooling for headless keyring selection);
  without this the resolved value, passed as an explicit flag, would
  override the caller's env configuration.

- Update --keyring-backend help text and the migration-scripts user guide,
  which still advertised "default test"; document the full resolution order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@akobrin1
akobrin1 requested a review from a-ok123 July 17, 2026 02:42
@akobrin1
akobrin1 merged commit 968e021 into master Jul 17, 2026
24 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.

2 participants