Skip to content

fix(sequencer): structured JSON errors for skipped transactions#2871

Open
0xtristan wants to merge 7 commits into
Sovereign-Labs:nightlyfrom
bulletxyz:fix/unsuccessful-tx-structured-json-errors
Open

fix(sequencer): structured JSON errors for skipped transactions#2871
0xtristan wants to merge 7 commits into
Sovereign-Labs:nightlyfrom
bulletxyz:fix/unsuccessful-tx-structured-json-errors

Conversation

@0xtristan

Copy link
Copy Markdown

Summary

When a transaction is skipped (e.g. due to a bad generation number or nonce), the sequencer was serializing the TransactionReceipt using format!("{:?}", receipt) — a Rust debug string stuffed into a JSON field. This made it impossible for SDK clients to deserialize the error in a structured way; they had to regex-match an opaque string.

TransactionReceipt, SkippedTxContents, and TxProcessingError all already derived serde::Serialize, so the fix was straightforward.

Commit 1 — serialize receipt as JSON

Replaces format!("{:?}", receipt) with serde_json::to_value(&receipt) in the UnsuccessfulTransaction arm of block_executor.rs. The Reverted case already handled this correctly; the Skipped/Ignored cases did not.

Commit 2 — CheckUniquenessError typed enum

TxProcessingError::CheckUniquenessFailed(String) gave clients no machine-readable way to identify the failure type or extract fields. This replaces the String with a proper enum:

pub enum CheckUniquenessError {
    BadGeneration { latest_generation: u64, provided_generation: u64 },
    DuplicateGeneration { generation: u64 },
    GenerationCapacityExceeded { current_generation: u64, next_valid_generation: u64 },
    BadNonce { expected_nonce: u64, provided_nonce: u64 },
    NonceTooLow { minimum_nonce: u64, provided_nonce: u64 },
    Internal(String), // state-access failures
}

The enum uses #[serde(tag = "kind", rename_all = "snake_case")] so the JSON output is fully typed:

{"kind": "bad_generation", "latest_generation": 1778774165791000, "provided_generation": 1778806984782}

The change threads through: generations.rs / nonces.rsUniqueness::check_uniquenessTransactionAuthorizer trait → StandardProvenRollupCapabilitiesregistered.rs / unregistered.rs. All test assertions are updated to match on variant fields rather than substrings.

Test plan

  • Existing uniqueness integration tests pass (sov-uniqueness)
  • nonce_buffer_task tests pass
  • tx_revert_tests pass
  • Verify JSON error response shape from a live sequencer for a bad-generation tx

🤖 Generated with Claude Code

0xtristan and others added 2 commits May 15, 2026 12:05
… string

`UnsuccessfulTransaction` was using `format!("{:?}", receipt)` for the
Skipped/Ignored cases, producing an unstructured Rust debug string in
the HTTP error body. `TransactionReceipt`, `SkippedTxContents`, and
`TxProcessingError` all already derive `serde::Serialize`, so switching
to `serde_json::to_value` is a one-line fix.

This matters for SDK clients that need to detect and recover from errors
like `CheckUniquenessFailed` — previously they had to regex-match a debug
string; now they can deserialize a typed JSON object.

A follow-up should add a `SkippedTxErrorDetails` struct (mirroring the
existing `AcceptTxErrorDetails` pattern) with stable machine-readable
codes and typed fields (e.g. `latest_generation: u64`) so clients get
fully structured data without any string parsing at all.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`TxProcessingError::CheckUniquenessFailed(String)` gave clients no
machine-readable way to identify the error type or extract fields like
the latest known generation. Clients had to regex the debug string.

This replaces it with `CheckUniquenessError`, a typed enum covering:
- `BadGeneration { latest_generation, provided_generation }`
- `DuplicateGeneration { generation }`
- `GenerationCapacityExceeded { current_generation, next_valid_generation }`
- `BadNonce { expected_nonce, provided_nonce }`
- `NonceTooLow { minimum_nonce, provided_nonce }`
- `Internal(String)` catch-all for state-access failures

The enum derives `serde::Serialize` with `tag = "kind"` so the JSON
output looks like:
  {"kind":"bad_generation","latest_generation":…,"provided_generation":…}

Combined with the previous commit serializing the receipt as JSON instead
of a Debug string, SDK clients can now fully deserialize and react to
uniqueness failures without any string parsing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@0xtristan
0xtristan marked this pull request as ready for review May 15, 2026 02:38
@0xtristan
0xtristan marked this pull request as draft May 15, 2026 02:38
0xtristan and others added 3 commits May 15, 2026 12:43
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rnal

The check_nonce_uniqueness and check_generation_uniqueness functions return
Result<(), CheckUniquenessError>, but StateReader::get() returns
Result<_, StateReader::Error> which has no From impl for CheckUniquenessError.
Explicitly map state access errors to Internal since there is no blanket
From<E: Error> like anyhow::Error has.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Arm 1 (Reverted) returns JsonObject (Map<String,Value>) via json_obj!/error_detail.
Arm 2 previously called serde_json::to_value which returns Value, not JsonObject.
Replace unwrap_or_else with a match that extracts the Object variant directly,
falling back to a json_obj error on serialization failure or a non-object result.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@0xtristan
0xtristan marked this pull request as ready for review May 15, 2026 03:03
-D warnings treats unused variables as errors in CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_ => json_obj!({
"error": format!("{:?}", receipt),
}),
_ => match serde_json::to_value(&receipt) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets impl ErrorDetail on TxProcessingError & CheckUniquenessError, then here we do something similar to reverted:

  sov_rollup_interface::stf::TxEffect::Skipped(skipped) => {
      skipped.error.error_detail().unwrap_or(json_obj!({}))
  }

In the impl on TxProcessingError for the CheckUniquenessFailed variant we delegate to inner.error_detail() for all other variants fall back to json_obj!({"error": format!("{:?}", receipt)})

Then we have everything in place to gradually migrate the other variants on TxProcessingError to structured errors

gas_used: <S::Gas as Gas>::zero(),
error: TxProcessingError::CheckUniquenessFailed(error_msg),
error: TxProcessingError::CheckUniquenessFailed(
sov_modules_api::CheckUniquenessError::BadNonce {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be possible to include a reason field on the error here so we can supply InvalidNonceReason? That enum provides a lot of useful context for exactly why the nonce is bad

/// Structured error returned when a transaction's uniqueness check fails.
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize, thiserror::Error)]
#[serde(rename_all = "snake_case", tag = "kind")]
pub enum CheckUniquenessError {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it be possible to define this closer to the uniqueness code? This file contains more higher level tx receipt types

Also we should impl ErrorDetail for it, which I think could jsut be:

impl ErrorDetail for CheckUniquenessError {
    fn error_detail(&self) -> Result<ErrorContext, Box<dyn std::error::Error + Send + Sync>> {
        Ok(err_detail!(self))
    }
}

let nonce = self
.nonces
.get(credential_id, state)
.map_err(|e| CheckUniquenessError::Internal(e.to_string()))?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have a CoreModuleError type that we can use for state accessor errors, so this becomes:

.map_err(CoreModuleError::state_read)?

And we add a CoreModuleError variant to CheckUniquenessError - see bank Error impl for example of how it's used

- Replace CheckUniquenessError::Internal(String) with Internal(CoreModuleError)
  so state-read failures serialize as structured JSON instead of a bare string.
  Add Clone, PartialEq, Eq, Deserialize to CoreModuleError to keep downstream
  derives intact (all lossy, reconstructing via Display/Generic).

- Add ErrorDetail impls for CheckUniquenessError and TxProcessingError.
  Update block_executor.rs to use error_detail() for both Reverted and Skipped
  arms; simplify the catch-all _ arm.

- Use CoreModuleError::state_read in nonces.rs and generations.rs instead of
  map_err(|e| Internal(e.to_string())).

- Add BadNonceReason enum to carry the nonce queue rejection context (Timeout,
  Replaced, AlreadyQueued, etc.) through to the CheckUniquenessError::BadNonce
  variant. Restore the test assertion that was weakened to { .. } by threading
  the expected reason through Outcome::Err and checking it against the error.

- Restore the generation cutoff comment header in generations.rs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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