Skip to content

fix(managed-wallet): answer 400 when the chain refuses a message as invalid in simulation - #4544

Merged
baktun14 merged 2 commits into
mainfrom
fix/managed-wallet-answer-400-to-invalid-messages
Oct 21, 2026
Merged

baktun14 merged 2 commits into
mainfrom
fix/managed-wallet-answer-400-to-invalid-messages

Conversation

@baktun14

@baktun14 baktun14 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

POST /v1/tx answered 500 five times on 2026-10-21 for transactions whose gas simulation the chain refused at message validation:

Query failed with (6): rpc error: code = Unknown desc = invalid pubkey value with gas used: '1150': unknown request
Query failed with (6): rpc error: code = Unknown desc = invalid deposit with gas used: '1150': unknown request

The chain runs message validation before the ante handler and returns the error as is, so the log never says failed to execute message. tx-signer only answers 400 when it does (#4052), so these went out as 500. console-api relayed them as "Service temporarily unavailable" and both 5xx alerts fired.

Fixes CON-1343

What

  • Gas estimation also raises MessageExecutionFailedError when the Simulate rejection's description opens with "invalid". The error handler already answers that error with 400 and the chain's message.
  • The match is anchored to the start of the description. Ante refusals the node raises on its own (an unordered ttl beyond the node's last block time ends in : invalid request) and recovered chain panics (recovered: runtime error: invalid memory address ...) stay 500.
  • Sequence mismatches, expired simulations and transport failures keep their current answers.
  • console-api needs no change: /v1/tx already relays a tx-signer 400 for caller-supplied messages (fix(managed-wallet): return the signer's 400 or 403 to the /v1/tx caller instead of a 500 #4057), and messages console builds itself still fail as 500.

Testing

  • Unit tests cover three validation refusals, plus an unordered ttl refusal, a recovered panic and a node-level query refusal that must stay unclassified. A functional test has the mocked node refuse Simulate with invalid deposit and checks for a 400 with nothing broadcast.
  • npm test, lint and tsc pass in apps/tx-signer. Stryker kills 5 of 5 mutants on the changed lines.

Summary by CodeRabbit

  • Bug Fixes
    • Improved transaction simulation error classification for invalid messages and requests.
    • Preserved detailed chain error messages when simulation rejects a transaction.
    • Ensured invalid transactions return HTTP 400 responses without being broadcast.
    • Continued handling expiration, excessive TTL, recovered panics, and other refusal cases appropriately.

…nvalid in simulation

A message the chain refuses at validation reaches gas simulation as its own error, unwrapped,
so it never says "failed to execute message" and was answered with a 500. Simulate rejections
whose description calls the transaction invalid now raise MessageExecutionFailedError too.
Sequence mismatches and expired simulations keep their current answers.

Refs CON-1343
@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.26%. Comparing base (05f0098) to head (7b52d43).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4544      +/-   ##
==========================================
+ Coverage   94.07%   94.26%   +0.19%     
==========================================
  Files        1773     1697      -76     
  Lines       50951    48785    -2166     
  Branches    12442    11994     -448     
==========================================
- Hits        47934    45989    -1945     
+ Misses       2707     2501     -206     
+ Partials      310      295      -15     
Flag Coverage Δ *Carryforward flag
api 96.03% <ø> (ø) Carriedforward from d5fd3e0
deploy-web 92.91% <ø> (ø) Carriedforward from d5fd3e0
log-collector ?
notifications 95.95% <ø> (ø) Carriedforward from d5fd3e0
provider-console 81.68% <ø> (ø) Carriedforward from d5fd3e0
provider-inventory ?
provider-proxy 92.21% <ø> (ø) Carriedforward from d5fd3e0
tx-signer 93.43% <100.00%> (+0.01%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...-client-factory/signing-stargate-client.factory.ts 96.38% <100.00%> (+0.04%) ⬆️

... and 76 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude claude 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.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@pullfrog

pullfrog Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

This run hit its 1h time limit while still working. Give runs longer with --timeout=2h (or --notimeout) in this repo's instructions, or raise the workflow's timeout input if it sets one, since that input wins. A smaller PR or a lower effort also finishes sooner.

Repo settings → · Flags →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using GLM-5.3

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository: akash-network/console/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 0cc6937a-2e5d-411a-958c-70f45b434d90
📥 Commits

Reviewing files that changed from the base of the PR and between 05f0098 and 7b52d43.

📒 Files selected for processing: 3
  • apps/tx-signer/src/lib/signing-stargate-client-factory/signing-stargate-client.factory.spec.ts
  • apps/tx-signer/src/lib/signing-stargate-client-factory/signing-stargate-client.factory.ts
  • apps/tx-signer/test/functional/sign-and-broadcast-tx.spec.ts
+----------------------------------------------------------------------------------------------------------------------------------------------+
| Eliminate effects between unrelated things. Design components that are self-contained, independent, and have a single, well-defined purpose. |
+----------------------------------------------------------------------------------------------------------------------------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Ante refusals such as an unordered ttl the node's clock rejects end in a generic
": invalid request", and a recovered chain panic mentions an invalid memory address.
Matching "invalid" anywhere turned those node-side failures into 400s, so the match
is now anchored to the start of the description.

Refs CON-1343
@baktun14
baktun14 added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 3211eab Oct 21, 2026
76 checks passed
@baktun14
baktun14 deleted the fix/managed-wallet-answer-400-to-invalid-messages branch October 21, 2026 16:10

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes

Re-reviewed the delta since the previously reviewed d5fd3e0 — the single new commit 7b52d43, which tightens the invalid-message match to descriptions that open with "invalid".

  • Anchored INVALID_MESSAGE_RE to the start of the description — a Simulate rejection now classifies as MessageExecutionFailedError only when its desc opens with invalid (desc = invalid\b). The three incident wordings stay 400 (invalid deposit, invalid pubkey value, Invalid groups via the /i flag), while ante refusals that merely end in a generic : invalid request (an unordered TTL past the node's 10-minute cap, … has already passed) and recovered chain panics keep answering 500. This supersedes the earlier optional suggestion to bound UNORDERED_TX_TTL_MS in env.config.ts by fixing the misclassification class itself.
  • Added negative tests for both exclusion cases — a new it.each asserts the unordered-TTL and recovered-panic refusals stay unclassified with the chain's wording preserved and a single simulate attempt (no retry); the node-level refusal test also gained a no-retry assertion.

The fixture wording was verified against cosmos-sdk v0.53.4 (x/auth/ante/sigverify.go): verifyUnorderedNonce raises Wrapf(ErrInvalidRequest, "unordered tx ttl exceeds %s", …), which renders detail-first and ends : invalid request, so the anchor is the right discriminator; the ErrTxDecode/ErrUnauthorized wraps that do open with "invalid" are unreachable for cosmjs-built proto txs. Full tx-signer battery green locally: 179 tests (160 unit + 19 functional), npx tsc --noEmit, lint and prettier clean.

Pullfrog  | View workflow run | Using GLM-5.3 | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant