Skip to content

feat: remove SIGN_MODE_TEXTUAL#26456

Merged
aljo242 merged 6 commits into
mainfrom
cozart/remove-sign-mode-textual
Jun 8, 2026
Merged

feat: remove SIGN_MODE_TEXTUAL#26456
aljo242 merged 6 commits into
mainfrom
cozart/remove-sign-mode-textual

Conversation

@aljo242

@aljo242 aljo242 commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Removes all implementation code, protos, tests, and tooling for SIGN_MODE_TEXTUAL
  • Reserves proto enum value 2 and string "SIGN_MODE_TEXTUAL" per protobuf best practice to prevent future reuse
  • Marks ADR-050 and its annexes as ARCHIVED

Changes

  • Deleted x/tx/signing/textual/ (~4,000 lines: all renderers, CBOR encoder, testdata, fuzz corpus, internal proto)
  • Deleted proto/cosmos/msg/textual/v1/textual.proto and generated api/ file
  • Deleted x/auth/tx/textual.go, x/auth/tx/config/expected_keepers.go
  • Removed TextualCoinMetadataQueryFn wiring from ConfigOptions and all simapp root.go files
  • Removed SIGN_MODE_TEXTUAL cases from APISignModeToInternal, internalSignModeToAPI, SignWithLedger, client flag help text, and tx factory
  • Removed Ledger+textual availability check from client/cmd.go

Test plan

  • go test -race ./... passes (verified locally)
  • golangci-lint run ./... passes (verified locally)
  • No CHANGELOG entry yet — add before merge

🤖 Generated with Claude Code

@github-actions

This comment has been minimized.

@greptile-apps

greptile-apps Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the SIGN_MODE_TEXTUAL feature in its entirety — approximately 4,000 lines deleted — including all renderers, the CBOR encoder, test data, the internal textual.proto, and the x/auth/tx/textual.go wiring. The proto enum value 2 and its string name are reserved to prevent future reuse, and ADR-050 with its annexes is archived.

  • Deletes x/tx/signing/textual/, x/auth/tx/textual.go, x/auth/tx/config/expected_keepers.go, and proto/cosmos/msg/textual/v1/textual.proto; removes TextualCoinMetadataQueryFn from all simapp and enterprise root.go files.
  • Relocates the coin.json / coins.json test fixtures from x/tx/signing/textual/internal/testdata/ to core/coins/internal/testdata/ so TestFormatCoin / TestFormatCoins continue to pass.
  • Adds a CHANGELOG breaking-change entry; five store-package generated files (store/internal/kv/kv.pb.go and siblings) also have their binary descriptors updated, though their source .proto files are not part of this diff.

Confidence Score: 5/5

Safe to merge — the removal is thorough, the reserved proto value prevents future misuse, and all compilation-breaking references have been cleaned up.

All Go constants for SIGN_MODE_TEXTUAL are removed, no dangling imports remain, the coin-format testdata was properly relocated, and the proto enum correctly uses reserved for value 2. The only notable items are descriptor-only changes in five unrelated store files — neither affects runtime correctness.

The five store/*.pb.go files with updated descriptors deserve a second look to confirm they were intentionally regenerated alongside the signing proto changes.

Important Files Changed

Filename Overview
x/auth/tx/config/config.go Removes MetadataBankKeeper injection, SIGN_MODE_TEXTUAL enablement logic, and the two public helpers NewBankKeeperCoinMetadataQueryFn / NewGRPCCoinMetadataQueryFn; remaining wiring is clean.
x/auth/tx/config.go Removes TextualCoinMetadataQueryFn from ConfigOptions and the SIGN_MODE_TEXTUAL case from NewSigningHandlerMap; empty ConfigOptions{} correctly falls back to DefaultSignModes.
proto/cosmos/tx/signing/v1beta1/signing.proto Correctly reserves enum value 2 and the string SIGN_MODE_TEXTUAL per protobuf best practices to prevent future reuse.
simapp/simd/cmd/root.go Removes the online-mode GRPC textual TxConfig setup; the remaining client context initialisation is unaffected.
store/internal/kv/kv.pb.go Binary file descriptor bytes changed (217→232 bytes comment) with no corresponding proto source change in this diff; same pattern appears in store/snapshots, store/streaming, and store/types — these look like incidental regeneration from a make proto-all run.
core/coins/format_test.go ReadFile paths updated to internal/testdata/; new coin.json and coins.json files are present in the PR, so both tests will pass.
x/auth/signing/verify.go Removes SIGN_MODE_TEXTUAL cases from APISignModeToInternal and internalSignModeToAPI; any incoming tx carrying sign-mode value 2 now correctly falls through to the unsupported default error.
client/cmd.go Removes the Ledger+textual availability guard and the textual exemption from the Ledger amino-JSON fallback; the remaining Ledger code path is logically consistent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CLI --sign-mode flag] --> B{Parse sign mode string}
    B -->|direct| C[SIGN_MODE_DIRECT]
    B -->|amino-json| D[SIGN_MODE_LEGACY_AMINO_JSON]
    B -->|direct-aux| E[SIGN_MODE_DIRECT_AUX]
    B -->|eip-191| F[SIGN_MODE_EIP_191]
    B -->|textual REMOVED| G[No case matched - SIGN_MODE_UNSPECIFIED]
    B -->|unknown| G
    C --> H{Ledger key?}
    D --> H
    E --> H
    F --> H
    H -->|yes, not amino-json and no protobuf| I[Force SIGN_MODE_LEGACY_AMINO_JSON]
    H -->|no| J[Use selected sign mode]
    I --> J
    J --> K{HandlerMap.GetSignBytes}
    K -->|DIRECT/AMINO/DIRECT_AUX/EIP_191| L[Sign and broadcast]
    K -->|value 2 TEXTUAL reserved| M[unsupported sign mode error]
Loading

Reviews (2): Last reviewed commit: "chore: regenerate proto files via make p..." | Re-trigger Greptile

Comment thread core/coins/format_test.go
Comment thread types/tx/signing/signing.pb.go
@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.68%. Comparing base (71efc78) to head (4ae9c3d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #26456      +/-   ##
==========================================
- Coverage   65.00%   64.68%   -0.33%     
==========================================
  Files         814      798      -16     
  Lines       56655    55485    -1170     
==========================================
- Hits        36829    35889     -940     
+ Misses      19826    19596     -230     
Files with missing lines Coverage Δ
client/cmd.go 78.04% <ø> (+0.66%) ⬆️
client/flags/flags.go 81.81% <100.00%> (ø)
client/tx/factory.go 75.37% <ø> (+0.56%) ⬆️
crypto/keyring/keyring.go 76.62% <ø> (+0.61%) ⬆️
crypto/ledger/ledger_secp256k1.go 68.75% <ø> (ø)
simapp/app.go 91.99% <100.00%> (-0.08%) ⬇️
x/auth/signing/verify.go 59.25% <ø> (+4.08%) ⬆️
x/auth/tx/config.go 83.63% <ø> (+0.30%) ⬆️
x/auth/tx/config/config.go 87.50% <ø> (+30.65%) ⬆️
x/tx/signing/std/handler_map.go 0.00% <ø> (ø)

... and 4 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aljo242

aljo242 commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@greptile re-review

@Eric-Warehime Eric-Warehime left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be good to proactively add an entry to UPGRADING.md for this.

aljo242 added a commit that referenced this pull request Jun 8, 2026
Addresses Eric-Warehime's review comment on #26456.
Adds a checklist item, ToC entry, and migration guide for chains
that configured SIGN_MODE_TEXTUAL.
@aljo242 aljo242 enabled auto-merge June 8, 2026 15:50
aljo242 added 6 commits June 8, 2026 11:50
Removes all code, protos, tests, and documentation for the SIGN_MODE_TEXTUAL
signing mode. The proto enum value is reserved (reserved 2; reserved "SIGN_MODE_TEXTUAL")
per protobuf best practice. ADR-050 and its annexes are marked ARCHIVED.
Addresses Eric-Warehime's review comment on #26456.
Adds a checklist item, ToC entry, and migration guide for chains
that configured SIGN_MODE_TEXTUAL.
@aljo242 aljo242 force-pushed the cozart/remove-sign-mode-textual branch from b41e75f to 4ae9c3d Compare June 8, 2026 15:51
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🔒 WARNING: unsigned commits detected

This pull request contains 6 commit(s) without a verified signature.

How to fix:

  1. Set up commit signing (GPG or SSH).
  2. Amend/rebase so every commit in this PR is verified-signed.
  3. Push the updated branch and open a new PR, or ask a maintainer to reopen once fixed.

Docs: https://docs.github.com/authentication/managing-commit-signature-verification

Unsigned commits:

  • 377d846 — feat: remove SIGN_MODE_TEXTUAL
  • ab2d892 — chore: relocate coin testdata from deleted textual package
  • 3b09a31 — chore: add CHANGELOG entry for SIGN_MODE_TEXTUAL removal
  • 00e611e — chore: regenerate proto files via make proto-all
  • fe652c0 — chore: fix gci import ordering in x/auth/tx/config/config.go
  • 4ae9c3d — docs: add SIGN_MODE_TEXTUAL removal to UPGRADING.md

@aljo242 aljo242 merged commit 97a4ee0 into main Jun 8, 2026
46 checks passed
@aljo242 aljo242 deleted the cozart/remove-sign-mode-textual branch June 8, 2026 16:09
aljo242 added a commit that referenced this pull request Jun 8, 2026
…hase 4a)

Replaces cosmossdk.io/api/cosmos/tx/v1beta1.SignDoc with the gogoproto-
generated github.com/cosmos/cosmos-sdk/types/tx.SignDoc in the
SIGN_MODE_DIRECT handler. SignDoc has only scalar fields (bytes, string,
uint64) so gogoproto.Marshal produces the same deterministic bytes as
proto.MarshalOptions{Deterministic:true}.Marshal.

Remaining txv1beta1 usage is architecturally blocked by the decode
pipeline:
- decode/decode.go: needs ProtoReflect().Descriptor() for
  RejectUnknownFieldsStrict
- tx_data.go / directaux: TxData.Body.Messages is []*anypb.Any;
  anyutil.Unpack requires *anypb.Any specifically
- adapter.go / testutil / aux_builder: build TxData or SignerInfo that
  depend on the above types

Also removes the now-unused txsigning import from client/cmd.go (the
SIGN_MODE_TEXTUAL check it guarded was removed by #26456).

Issue: N/A
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