fix(submit): robust nonce recovery via chain re-query

[pthmas]

Summary

Fixes fragile sequence mismatch handling in DirectSubmitter (closes #8).

The previous implementation detected ErrWrongSequence by scanning error strings and recovered by regex-parsing the expected sequence number out of the message. This broke silently if Celestia changed error message wording.

Changes

• checkTxStatus: detect sequence mismatch via ABCI code 32 instead of string pattern matching — stable across node versions
• recoverSequenceLocked: re-query chain via AccountInfo on mismatch instead of parsing the expected sequence from the error string; signature changes to (ctx context.Context) error
• broadcastTx: thread ctx through to recovery; add attempt number and address to mismatch error context
• maxSequenceRetryRounds: increased from 2 → 3 per spec
• Removed: expectedSequenceFromMismatchText and strconv import — no longer needed
• Kept: isSequenceMismatchText for the gRPC error path (when BroadcastTx returns a non-nil error rather than a TxStatus)

New tests:

• TestDirectSubmitterMismatchDetectedByABCICode — code 32 with no parseable log triggers retry
• TestDirectSubmitterMismatchReQueryFails — re-query error surfaces correctly
• TestDirectSubmitterExhaustsAllRetries — confirms 3 attempts then returns errSequenceMismatch

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved reliability of transaction broadcasting by refining how sequence-related failures are detected and recovered.
  • Increased maximum retry attempts to enhance transaction submission success rates.
  • Enhanced error messages with additional context during transaction recovery.

• Tests

  • Expanded test coverage for transaction submission failure scenarios and edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactored sequence mismatch recovery in direct transaction submission. Replaced error text parsing with ABCI code detection (Code 32). Introduced recoverSequenceLocked method for account state re-queries. Increased max retry rounds from 2 to 3. Added comprehensive test coverage for recovery paths.

Changes

Cohort / File(s)	Summary
Sequence Mismatch Recovery Logic pkg/submit/direct.go	Introduced explicit attempt index in retry loop; changed mismatch detection from RawLog text inspection to tx.Code == 32 check; replaced inline sequence parsing with new recoverSequenceLocked(ctx) error method that invalidates cache, re-queries account state, and updates sequence values; removed expectedSequenceFromMismatchText helper and related strconv dependency; wrapped mismatch errors with attempt number and signer address context.
Sequence Recovery Tests pkg/submit/direct_test.go	Enhanced sequenceRecoveryAppClient test double with accountSequenceQueue field to return queued sequence values via AccountInfo; updated existing retry/recovery test assertions to expect increased accountCalls count; added three new test cases: TestDirectSubmitterMismatchDetectedByABCICode (ABCI-based detection without parseable log), TestDirectSubmitterMismatchReQueryFails (re-query failure handling), and TestDirectSubmitterExhaustsAllRetries (retry exhaustion with repeated mismatches).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• add test cases to build robustness #48: Directly related; previous PR introduced expectedSequenceFromMismatchText and per-signer sequencing, which this PR removes in favor of ABCI code-based detection and centralized recovery via recoverSequenceLocked.

Poem

🐰 Hop, hop, sequence flows so true,
No regex parsing—Code 32 will do!
Requery swift when mismatches appear,
Three retry rounds bring hope and cheer! ✨

🚥 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 title accurately reflects the main change: implementing robust sequence number recovery via chain re-query instead of error string parsing.
Linked Issues check	✅ Passed	The PR implements core objectives from issue #8: sequence mismatch detection via ABCI code 32, re-query recovery without regex parsing, increased retry rounds to 3, and error wrapping with context.
Out of Scope Changes check	✅ Passed	All changes are directly related to issue #8 objectives: sequence mismatch handling, recovery mechanism, retry logic, and supporting test coverage for these features.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

pkg/submit/direct.go (1)

462-464: Consider extracting ABCI code 32 as a named constant.

While code 32 is the well-known Cosmos SDK code for sequence mismatch, extracting it as a named constant would improve readability and make it easier to find all sequence-mismatch related logic.

💡 Suggested constant definition

 const (
 	defaultFeeDenom        = "utia"
 	defaultPollInterval    = time.Second
 	maxSequenceRetryRounds = 3
+	// abciCodeSequenceMismatch is the Cosmos SDK ABCI code for account sequence mismatch.
+	abciCodeSequenceMismatch = 32
 )

Then at line 462:

-	if tx.Code == 32 {
+	if tx.Code == abciCodeSequenceMismatch {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/submit/direct.go` around lines 462 - 464, Extract the literal ABCI code
32 into a named constant (e.g., SequenceMismatchCode = 32) and replace the magic
number check in the submit logic (the tx.Code == 32 comparison) with that
constant; update the error-return branch that references errSequenceMismatch so
it uses the new constant for clarity and to centralize sequence-mismatch logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/submit/direct.go`:
- Around line 462-464: Extract the literal ABCI code 32 into a named constant
(e.g., SequenceMismatchCode = 32) and replace the magic number check in the
submit logic (the tx.Code == 32 comparison) with that constant; update the
error-return branch that references errSequenceMismatch so it uses the new
constant for clarity and to centralize sequence-mismatch logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0fe5c5b-f748-409d-8d7a-9c915780e180

📥 Commits

Reviewing files that changed from the base of the PR and between b88cd3f and a578d63.

📒 Files selected for processing (2)

• pkg/submit/direct.go
• pkg/submit/direct_test.go