fix(evmigration): keyring-backend auto-resolution and os-backend prompt fix for migration scripts#193
Merged
Merged
Conversation
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>
There was a problem hiding this comment.
💡 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".
…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>
a-ok123
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes to the EVM-migration shell tooling, found while debugging a live validator migration that appeared to hang:
1.
oskeyring backend hung with a hidden passphrase prompt (aa453270)The passphrase-prompt stderr-tee in
lumerad_tx/preview_tx_body/resolve_addresswas gated onKEYRING_BACKEND == "file". But cosmos-sdk'sosbackend does not pinAllowedBackends, 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 testby default (2f680412..3b0009a7)When
--keyring-backendis not passed,resolve_keyring_backendnow resolves it the waylumeradusers expect, first hit wins:--keyring-backendflagkeyring-backendfrom<home>/config/client.toml(--homeselects home;--keyring-dirdoes not moveclient.toml)--keyring-dir(else home):keyring-test/→test,keyring-file/→fileos— the SDK defaultThe chosen value and its source are logged before anything signs. Wired into
migrate-account.sh,migrate-validator.sh, and all threemigrate-multisig.shsubcommands (resolved value propagates to child invocations as an explicit flag). Existing devnet/CI flows keep working viakeyring-test/disk detection.Design + plan:
docs/design/2026-07-16-keyring-backend-resolution-{design,plan}.md.Also fixed along the way (
5001452d):resolve_keyring_backendcrashed underset -uwhen$HOMEis unset (systemd/cron runs) instead of reaching theosfallback.common.bats— failing tests died silently with nonot okdiagnostics.setup()now saves/restores bats' trap.Housekeeping (
c3557ec6,9f30f048): devnet-modulex/crypto0.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-$HOMEregression,os-prompt tee regression, client.toml e2e for account + multisig submit; gas-helper tests ported from the deleted stalescripts/evmigration-common_test.sh)make lint— 0 issuesmigrate-account.sh --dry-runagainst the shim with empty--homelogskeyring backend: os (SDK default; ...)🤖 Generated with Claude Code