Skip to content

fix: reconcile account info on broadcast failure#65

Open
Vritra4 wants to merge 4 commits into
mainfrom
fix/refresh-accinfo
Open

fix: reconcile account info on broadcast failure#65
Vritra4 wants to merge 4 commits into
mainfrom
fix/refresh-accinfo

Conversation

@Vritra4
Copy link
Copy Markdown
Contributor

@Vritra4 Vritra4 commented Jun 4, 2026

  • Add AccountSequenceTracker to reconcile sequence on broadcast failure, recovering from account sequence mismatch without a restart
  • bump initia.js

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced account sequence tracking with improved error recovery during transaction broadcasts.
    • Better handling of blockchain sequence mismatch scenarios.
  • Improvements

    • Optimized IBC channel state and ordering handling.
    • Refined transaction error reconciliation logic.
  • Chores

    • Updated @initia/initia.js dependency to v1.1.0.
    • Added comprehensive test coverage for sequence management and error scenarios.

@Vritra4 Vritra4 self-assigned this Jun 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Walkthrough

This PR refactors wallet transaction handling to cache and reconcile blockchain account sequence state. It updates IBC channel types in the REST client with explicit enum mappings, introduces an AccountSequenceTracker utility for managing sequence caching and error reconciliation, integrates the tracker into WalletWorker, and adds comprehensive integration tests validating sequence reconciliation across multiple failure scenarios.

Changes

Account Sequence Tracking and Reconciliation

Layer / File(s) Summary
REST Client IBC Channel Type and Enum Mapping
package.json, src/lib/restClient.ts
Updated @initia/initia.js to ^1.1.0. REST client now maps raw channel state and ordering strings to State/Order proto enums using nullish coalescing (??). ChannelResponse.channel type redefined locally with explicit enum fields instead of Channel.Data reference. Channel import removed from @initia/initia.js.
REST Client Channel Tests
src/lib/restClient.spec.ts
Added Order and State proto enum imports and table-driven tests verifying correct mapping of raw channel state/ordering strings to enums, passthrough of version/connection_hops/counterparty fields, unknown value mapping to UNRECOGNIZED, and preservation of zero-value enum members.
Channel Open Try Message Generation
src/msgs/channelOpenTry.ts, src/msgs/channelOpenTry.spec.ts
Updated generateMsgChannelOpenTry to import State from protobuf path and set explicit upgrade_sequence: 0 for newly opened channels. Added test verifying STATE_TRYOPEN, ORDER_UNORDERED, upgrade_sequence: 0, connection_hops, and counterparty_version fields.
MSW Test Server Lifecycle Management
src/test/testSetup.ts
Captures setupServer in module-scoped server variable with selective unhandled request bypass based on hostname regex pattern. Wires server startup in beforeAll() and shutdown/reset in afterAll().
AccountSequenceTracker Utility
src/workers/accountSequence.ts
New utility class caching blockchain account sequence/account number with conditional initialization via injected fetcher. Includes parseExpectedSequenceFromRawLog() helper extracting sequence from transaction logs; markBroadcastSuccess() increments cached sequence; reconcileTxError(rawLog?) parses expected sequence with fallback refresh; reconcileBroadcastException() always refreshes.
AccountSequenceTracker Tests
src/workers/accountSequence.spec.ts
Tests verify initialization call count, local sequence increment on broadcast success, sequence reconciliation on log-parsed expected sequence without extra queries, chain refresh for non-sequence errors, and sequence update after broadcast exceptions.
Wallet Worker Sequence Tracker Integration
src/workers/wallet.ts
Replaces inline sequence/accountNumber fields with AccountSequenceTracker instance. Constructor adds optional options.autoStart flag for conditional auto-start (defaults to true). Removes initAccInfo() helper. Adds signAndBroadcast(msgs) method centralizing signing, broadcasting, sequence tracker initialization, and reconciliation via reconcileTxError and reconcileBroadcastException. handlePackets calls signAndBroadcast and logs e.response.data on error.
Sequence Reconciliation Integration Tests
src/workers/walletSequence.spec.ts
Comprehensive test suite with logger mock and wallet worker test double; verifies sequence handling in success path (no extra query), sequence mismatch (uses parsed expected sequence), non-sequence error (chain refresh), broadcast timeout (sequence refresh and warning log), and reconciliation query failure (correct call counts).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant WalletWorker
  participant AccountSequenceTracker
  participant Chain
  Client->>WalletWorker: handlePackets(messages)
  activate WalletWorker
  WalletWorker->>AccountSequenceTracker: ensureInitialized()
  activate AccountSequenceTracker
  AccountSequenceTracker->>Chain: fetch account sequence
  deactivate AccountSequenceTracker
  WalletWorker->>WalletWorker: signAndBroadcast(msgs)
  WalletWorker->>Chain: broadcast signed tx
  alt Broadcast Success
    WalletWorker->>AccountSequenceTracker: markBroadcastSuccess()
  else TxError with Log
    WalletWorker->>AccountSequenceTracker: reconcileTxError(rawLog)
  else Broadcast Exception
    WalletWorker->>AccountSequenceTracker: reconcileBroadcastException()
  end
  deactivate WalletWorker
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 A sequence tracker hops with grace,
Caching numbers in its safe place,
When broadcasts stumble, logs reveal,
The truth of what should heal!
Reconciled with wallet's might,
Account state shines ever bright.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix: reconcile account info on broadcast failure' directly summarizes the main change: adding account sequence reconciliation on broadcast failures to prevent service restart.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/refresh-accinfo

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/workers/walletSequence.spec.ts (1)

68-77: ⚡ Quick win

Fail fast when mocked accountInfo responses are exhausted.

Returning undefined here can trigger indirect TypeErrors later (getSequenceNumber on undefined), which makes test failures harder to diagnose.

Proposed patch
   const authAccountInfo = jest
     .fn()
     .mockImplementation(() => {
       const response = options.accountInfoResponses.shift()
+      if (response === undefined) {
+        throw new Error('No mocked accountInfo response left')
+      }
       if (response instanceof Error) {
         return Promise.reject(response)
       }

       return Promise.resolve(response)
     })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/workers/walletSequence.spec.ts` around lines 68 - 77, The mock for
authAccountInfo currently returns undefined when options.accountInfoResponses is
exhausted, causing downstream TypeErrors; update the authAccountInfo mock (the
jest.fn() implementation that consumes options.accountInfoResponses) to detect
when shift() yields undefined and immediately return a rejected Promise (or
throw) with a clear error message like "No more mocked accountInfo responses" so
tests fail fast and show a clear cause instead of later getSequenceNumber on
undefined failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/workers/walletSequence.spec.ts`:
- Around line 68-77: The mock for authAccountInfo currently returns undefined
when options.accountInfoResponses is exhausted, causing downstream TypeErrors;
update the authAccountInfo mock (the jest.fn() implementation that consumes
options.accountInfoResponses) to detect when shift() yields undefined and
immediately return a rejected Promise (or throw) with a clear error message like
"No more mocked accountInfo responses" so tests fail fast and show a clear cause
instead of later getSequenceNumber on undefined failures.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c00d1e9b-bd1a-4221-aeef-ae09d8646c08

📥 Commits

Reviewing files that changed from the base of the PR and between 2b2f601 and 7c4d723.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • package.json
  • src/lib/restClient.spec.ts
  • src/lib/restClient.ts
  • src/msgs/channelOpenTry.spec.ts
  • src/msgs/channelOpenTry.ts
  • src/test/testSetup.ts
  • src/workers/accountSequence.spec.ts
  • src/workers/accountSequence.ts
  • src/workers/wallet.ts
  • src/workers/walletSequence.spec.ts

@Vritra4 Vritra4 requested a review from a team June 4, 2026 11:40
@Vritra4 Vritra4 marked this pull request as ready for review June 4, 2026 11:42
Copy link
Copy Markdown
Member

@beer-1 beer-1 left a comment

Choose a reason for hiding this comment

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

utACK

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