Skip to content

Conversation

@reecepbcups
Copy link
Contributor

@reecepbcups reecepbcups commented Dec 30, 2025

tldr; cosmos multisigs with ledgers break when signing due to the EIP-712 not taking into account the chain-id. Tested and this is breaking, required on the nodes

Description

Ledger multisig transactions using EIP-712 signing failed on-chain verification with signature verification failed: unauthorized, even though local validate-signatures succeeded. This does not allow a ledger to be apart of a multisig when WithKeyringOptions(evmkeyring.Option()). is used (CoinType 60 / EthSecpK1)

The EIP-712 encoding uses a global eip155ChainID variable (262144) that is set during app initialization via encoding.MakeConfig(evmChainID). When CLI commands like tx multi-sign or tx validate-signatures run, they create a temporary app with simtestutil.EmptyAppOptions{}, which causes the EVM chain ID to default to 262144 instead of the chain's actual EVM chain ID.

This meant:

  • During Ledger signing: The CLI correctly used the chain ID from the --chain-id flag (e.g., 1230263908) to construct the EIP-712 typed data
  • During verification: The EIP-712 reconstruction used the wrong global chain ID (262144), producing a different domain hash

Testing

just sh-testnet

intervald keys add ledger0 --ledger --account 0
intervald keys add ledger1 --ledger --account 1
intervald keys add ledger2 --ledger --account 2

intervald keys add ledger-multisig \
  --multisig "ledger0,ledger1,ledger2" \
  --multisig-threshold 2

LEDGER_MULTISIG_ADDR=$(intervald keys show ledger-multisig -a)
echo "Ledger multisig address: $LEDGER_MULTISIG_ADDR"

# Fund from a genesis account
intervald tx bank send acc0 $LEDGER_MULTISIG_ADDR 1000000000000000000token \
  --node $NODE --chain-id $CHAIN_ID --gas auto -y

# Verify balance
sleep 1
intervald q bank balances $LEDGER_MULTISIG_ADDR --node $NODE

# Generate unsigned tx
intervald tx bank send $LEDGER_MULTISIG_ADDR interval124gau98h2hkqdsve9vxgkj0rxkdhs5e9twpg4x 100000000token \
  --node $NODE --chain-id $CHAIN_ID --gas 500000 \
  --from $LEDGER_MULTISIG_ADDR --generate-only > /tmp/test_unsigned_ledger_only.json

# Sign with Ledger 0 (will show EIP-712 hashes)
intervald tx sign /tmp/test_unsigned_ledger_only.json \
  --from ledger0 \
  --multisig ledger-multisig \
  --chain-id $CHAIN_ID \
  --node $NODE \
  --ledger \
  --output-document /tmp/sig_ledger0.json
  
  # Sign with Ledger 1 (will show EIP-712 hashes)
intervald tx sign /tmp/test_unsigned_ledger_only.json \
  --from ledger1 \
  --multisig ledger-multisig \
  --chain-id $CHAIN_ID \
  --node $NODE \
  --ledger \
  --output-document /tmp/sig_ledger1.json
  
  intervald tx multi-sign /tmp/test_unsigned_ledger_only.json ledger-multisig \
  /tmp/sig_ledger0.json /tmp/sig_ledger1.json \
  --chain-id $CHAIN_ID \
  --node $NODE \
  --output-document /tmp/final_ledger_only.json

# Validate locally
intervald tx validate-signatures /tmp/final_ledger_only.json \
  --node $NODE --chain-id $CHAIN_ID

# Broadcast
intervald tx broadcast /tmp/final_ledger_only.json \
  --node $NODE --chain-id $CHAIN_ID

Before this fix it was impossible to submit any signature from a ledger (it works just fine for hotwallets though) via the multisig.


I have...

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • x ] targeted the main branch

@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.49%. Comparing base (84c2a0a) to head (55343a7).

Files with missing lines Patch % Lines
ethereum/eip712/encoding.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #918      +/-   ##
==========================================
- Coverage   65.49%   65.49%   -0.01%     
==========================================
  Files         318      318              
  Lines       22222    22232      +10     
==========================================
+ Hits        14555    14561       +6     
- Misses       6502     6504       +2     
- Partials     1165     1167       +2     
Files with missing lines Coverage Δ
ethereum/eip712/encoding.go 76.66% <66.66%> (-1.52%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MudDev
Copy link

MudDev commented Jan 4, 2026

I've been investigating an error that's similar when transacting over IBC using EOA (Keplr/Leap), but the IBC via CLI works fine, I am curious if this resolves that issue as well.

This error was introduced in either v0.5.0 or v0.5.1 (Recently upgraded from v0.4.2)

signature verification failed; please verify account number (X), sequence (X) and chain-id (CHAIN_ID): unauthorized

Update: I applied this change to my RPC node, and I got the same error, so the IBC error doesn't seem to be related.

@reecepbcups
Copy link
Contributor Author

reecepbcups commented Jan 5, 2026

@MudDev This does not get applied to the RPC node (it can but it doesn't affect it). it has to do with the signer when encoding. So the bug is within keplr / leap on signing with the wrong chain id. The chain id refactor in 0.5 has caused so many issues

@aljo242
Copy link
Contributor

aljo242 commented Jan 6, 2026

Looks good - can we get some tests for this functionality?

@vladjdk
Copy link
Member

vladjdk commented Jan 6, 2026

Agreed with @aljo242. It would be useful to have a happy path systemtest in the eip712 tests to make sure that this doesn't break again in the future.

@reecepbcups
Copy link
Contributor Author

reecepbcups commented Jan 6, 2026

I tested on a 4x validator cluster and I don't feel like writing any unit tests for this. Just contributing the patch to fix what was broken in 0.5 refactor

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.

4 participants