Conversation
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughChangesDurable mint side-effect flow
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
0xgleb
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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.
Inline comments:
In `@src/jobs/mod.rs`:
- Around line 15-26: The import block in mod.rs needs to follow the two-group
organization: keep std::time::Duration in the std group and all external crate
imports together in the second group, without splitting them into extra
sections. Also bring type_name into module scope instead of using a fully
qualified path later in the module; update the imports near the top of the file
and use the imported type_name wherever it is referenced in the module.
In `@src/mint/job.rs`:
- Around line 355-372: The callback flow in job handling is not retry-safe: if
`send_mint_callback` succeeds but `MintCommand::RecordCallbackSent` fails, the
job can be retried and resend the Alpaca callback. Update the `job.rs` logic
around `send_mint_callback` and `RecordCallbackSent` to add an external
idempotency guard keyed by the mint/tx identity, or persist a durable
outbox/state transition before invoking the callback so retries do not duplicate
the side effect.
- Line 86: The quantity conversion failure in the mint job should be treated as
a domain failure instead of a retryable infrastructure error. In the mint
processing flow around the `quantity.to_u256_with_18_decimals()` conversion,
catch `QuantityConversionError`, record the mint as failed via
`RecordMintFailed`, and then return `Ok(())` so apalis does not keep re-driving
the same non-recoverable job. Use the existing mint job handler and
`RecordMintFailed` path to keep the failure persisted and non-retryable.
- Around line 18-23: The imports in the top of the job module are split into
three groups because std::sync::Arc is separated from the other external crates.
Reorganize the imports in this module so all external crates stay together in
one group with no blank lines, then add a single blank line before any internal
crate imports; use the existing symbols like Arc, Address, SendError, Store,
Deserialize, Serialize, and warn to keep the grouping consistent.
In `@src/mint/mod.rs`:
- Around line 838-865: `handle_record_tokens_minted` currently records
`TokensMinted` without verifying the confirmed Fireblocks transaction, so a
stale confirm can be applied to the wrong submission. Update
`RecordTokensMinted` to carry `fireblocks_tx_id`, then in
`handle_record_tokens_minted` compare that id against the stored value in
`Self::FireblocksSubmitted` before calling `validate_issuer_request_id` and
emitting `MintEvent::TokensMinted`. If the ids do not match, return the
appropriate `MintError` instead of recording the event.
- Around line 948-952: The MintRetryStarted flow in mint/mod.rs is dropping the
failed predecessor state too early by transitioning MintingFailed to Minting
before a retry submission is durably recorded. Update the retry path around
MintRetryStarted handling so the failed chain metadata (failed_from, attempts,
and previous Fireblocks tx context) is preserved in the retry job/command, or
only emit MintRetryStarted after the retry submission has been successfully
recorded. Use the MintRetryStarted and MintingFailed handling logic in the mint
state transition code to locate and adjust the state update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d042d93-eb9b-4826-a0c9-317b9b8d5c1f
📒 Files selected for processing (8)
src/jobs/mod.rssrc/lib.rssrc/mint/api/confirm.rssrc/mint/cmd.rssrc/mint/job.rssrc/mint/mod.rstests/redemption.rstests/redemption_dust.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
- GitHub Check: build
- GitHub Check: test
- GitHub Check: static
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Use two-group import organization: Group 1 (external crates with no blank lines), one blank line separator, Group 2 (internal crate code with no blank lines) - forbidden: three+ groups, blank lines within groups, function-level imports except enum variant imports in function bodies
Never use fully-qualified paths for non-ambiguous types - import them at module top; when ambiguous (e.g. alloy::rpc::types::Log vs alloy::primitives::Log), use qualified imports like rpc_types::Log
Files:
tests/redemption_dust.rssrc/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
tests/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.rs: E2E tests must spin up the full HTTP service, use ONLY the public API as external consumer would, use Anvil for local blockchain, mock only truly external systems, assert via API responses and Anvil state; setup phase exception: may use direct SQL to seed event store only, no other tables
E2E tests should test happy paths only and use real blockchain (Anvil)
Files:
tests/redemption_dust.rstests/redemption.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Package by Feature, Not by Layer - organize by business feature/domain, never by language primitives or technical layers; forbidden module names include types.rs, error.rs, models.rs, utils.rs, helpers.rs, http.rs, dto.rs, entities.rs, services.rs as catch-all technical layers
Zero tolerance for panics in non-test code - forbidden: unwrap(), expect(), panic!(), unreachable!(), unimplemented!() except in test code; forbidden: panicking index operations, division without zero-check; required: use ? for propagation, Result/Option with explicit handling
Keep visibility as restrictive as possible - prefer pub(crate) over pub, private over pub(crate) to enable better dead-code detection and make scope explicit
Use Typed Values in Error types - store typed values, not string representations; forbidden: format!("{value:?}") or .to_string() to convert typed data into error-field strings; correct: store Address, B256, or typed IDs directly
Use #[from] for error variants instead of verbose .map_err() calls - let the compiler guide error variants by using ? as if all required variants exist, then add #[from] variants only for errors the compiler complains about
#[from] variant naming must be generic (mirroring source error type), not claim what operation failed - forbidden: ReadSecret(#[from] io::Error), ParseConfig(#[from] serde_json::Error); correct: Io(#[from] io::Error), Json(#[from] serde_json::Error)
Make invalid states unrepresentable using ADTs/enums to encode business rules - forbidden: types with all/most Option fields, multiple nullable fields that contradict; correct: enum variants for mutually exclusive states with state-specific data inside each variant
Parse, Don't Validate - use newtypes with private inner values and fallible smart constructors as the ONLY way to create constrained domain values; forbidden: separate validate() methods, raw primitives for constrained values, public fields/constructors bypassing validation; applies to API keys, emai...
Files:
src/jobs/mod.rssrc/mint/cmd.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
🧠 Learnings (8)
📚 Learning: 2026-02-12T03:36:52.079Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 112
File: src/mint/recovery.rs:78-117
Timestamp: 2026-02-12T03:36:52.079Z
Learning: In Rust recovery loops or similar retry patterns,INFO/WARN log statements that are immediately followed by return; or break; inside the loop body are acceptable because the loop runs at most once for the exit path and won't flood logs. The existing guideline about DEBUG/TRACE logs per iteration applies to repeated logging inside loops; use INFO or WARN for exit paths and avoid per-iteration DEBUG/TRACE logs in retry loops. When reviewing code, flag cases where a log level dominates per-iteration noise and suggest moving exit-logs outside the hot loop or gating by a condition, ensuring the log appears at non-repetitive times.
Applied to files:
tests/redemption_dust.rssrc/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
📚 Learning: 2026-02-13T14:15:21.049Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 117
File: src/lib.rs:569-579
Timestamp: 2026-02-13T14:15:21.049Z
Learning: In Rust code, within loops that spawn long-running background tasks (e.g., tokio::spawn) such as detectors, monitors, or services, INFO logs inside the loop body are acceptable if the loop is bounded and each iteration launches a persistent background worker. This acknowledges a significant operational event per iteration without causing excessive log noise. Apply this guidance when the loop has a clear exit condition and the spawned task persists beyond the iteration. If the loop is unbounded or spawns short-lived tasks, prefer lower log levels or structured tracing to avoid log flooding.
Applied to files:
tests/redemption_dust.rssrc/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
📚 Learning: 2026-03-19T15:53:49.678Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 130
File: src/config.rs:73-76
Timestamp: 2026-03-19T15:53:49.678Z
Learning: In Rust error enums, follow the AGENTS.md constraint that variant names “must be generic when using #[from]” only for enum variants annotated with #[from] (i.e., variants that derive the `From` conversion for the wrapped error type). For variants populated via manual error construction/handling (e.g., created in code paths using `.map_err(...)`), the naming constraint does not apply—use descriptive or categorical variant names to group related failure reasons (e.g., `HttpClient(Box<dyn Error + Send + Sync>)`).
Applied to files:
tests/redemption_dust.rssrc/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
📚 Learning: 2026-06-24T21:02:22.771Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 118
File: src/job.rs:0-0
Timestamp: 2026-06-24T21:02:22.771Z
Learning: In Rust, when implementing the `std::fmt::Display`/`std::Debug` traits (or the core `std::fmt::Formatter`-using `fmt` method), the conventional single-letter parameter name `f` for `fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result` should be allowed. Do not flag `f` as a violation of a “no single-letter variable names” guideline; this name is an established Rust idiom for `Formatter`.
Applied to files:
tests/redemption_dust.rssrc/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/mint/api/confirm.rssrc/lib.rssrc/mint/mod.rs
📚 Learning: 2026-02-17T22:13:31.572Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 122
File: tests/recovery.rs:1013-1139
Timestamp: 2026-02-17T22:13:31.572Z
Learning: In tests located under the tests/ directory, do not include log assertions or tracing-test related checks. Observability assertions (logs) belong in unit or integration tests, not E2E tests. E2E tests should verify public API responses, blockchain state, and external mocks; avoid adding #[traced_test] or logs_contain_at in E2E test files.
Applied to files:
tests/redemption_dust.rstests/redemption.rs
📚 Learning: 2026-03-20T13:19:14.952Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 131
File: tests/receipt_transfer.rs:156-156
Timestamp: 2026-03-20T13:19:14.952Z
Learning: For ST0x-Technology/st0x.issuance E2E tests under tests/**/*.rs, fixed-sleep waits (tokio::time::sleep with Duration::from_secs) are an accepted pattern to wait for observable conditions. Do not raise an issue for these fixed sleeps as isolated problems. If migrating to a polling/bounded-wait helper (e.g., a shared wait_for utility) is desired, treat it as an intentional cross-cutting refactor that must be applied across all E2E test files simultaneously, rather than changing one file at a time.
Applied to files:
tests/redemption_dust.rstests/redemption.rs
📚 Learning: 2026-02-17T22:17:07.514Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 122
File: tests/smoke.rs:1-1
Timestamp: 2026-02-17T22:17:07.514Z
Learning: In test files under tests/**/*.rs, crate-level lint suppression #![allow(clippy::unwrap_used)] is acceptable only when explicitly permitted by the user. E2E tests may benefit from using .unwrap() for immediate panic-on-failure feedback showing the actual value. Apply this guideline only to tests and ensure suppression is clearly documented and restricted to cases approved by the user, avoiding blanket suppression in non-test code.
Applied to files:
tests/redemption_dust.rstests/redemption.rs
📚 Learning: 2026-06-04T17:02:39.675Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 168
File: src/lib.rs:993-993
Timestamp: 2026-06-04T17:02:39.675Z
Learning: In `src/lib.rs` and `src/config.rs` for `st0x.issuance`, treat `Config::receipt_poll_interval` as hardcoded from the `RECEIPT_POLL_INTERVAL` constant (60s) set in `Env::into_config`, with no env/CLI override. As long as it remains hardcoded (so `0` is unreachable on production paths), do not flag `tokio::time::interval(config.receipt_poll_interval)` in `spawn_periodic_receipt_backfills` as a potential panic risk. It’s acceptable for tests or external callers to construct `Config` with `receipt_poll_interval = 0` and trigger a panic (use that panic as test feedback). Also, do not introduce a runtime `Config::validate()` method; follow the project rule in `AGENTS.md` (“Parse, Don't Validate”).
Applied to files:
src/lib.rs
🔇 Additional comments (6)
src/jobs/mod.rs (1)
61-69: LGTM!Also applies to: 92-102
src/lib.rs (1)
21-30: LGTM!Also applies to: 42-42, 348-361, 1354-1472
src/mint/cmd.rs (1)
1-1: LGTM!Also applies to: 69-82, 92-116
tests/redemption.rs (1)
285-285: LGTM!Also applies to: 335-340
tests/redemption_dust.rs (1)
318-318: LGTM!Also applies to: 417-417
src/mint/api/confirm.rs (1)
10-15: LGTM!Also applies to: 162-225, 236-236, 450-465, 519-519, 538-538
4b18156 to
b6d4d41
Compare
52b24fa to
eeac98b
Compare
eeac98b to
739382e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/jobs/mod.rs (1)
1-16: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep the interim job runtime inside the mint feature.
This crate-level
jobsmodule packages infrastructure by technical layer, while every current consumer is mint-specific. Move it under the mint feature untilevent-sorceryreplaces it, and update the now-stale single-consumer note.As per coding guidelines,
src/**/*.rsmust be organized by business feature/domain, never by technical layer.🤖 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/jobs/mod.rs` around lines 1 - 16, Move the interim job runtime from the crate-level jobs module into the mint feature’s module hierarchy, updating module declarations and imports so mint::recovery remains the consumer. Revise the module documentation to remove the stale crate-level/shared-consumer rationale and describe its mint-scoped interim purpose until event-sorcery provides the replacement.Source: Coding guidelines
src/lib.rs (1)
400-416: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPrevent recovery from racing the durable mint chain.
The immediate periodic reconciler can enqueue a
MintRecoveryJobfor a mint already owned by submit/confirm/callback queues. Both paths may perform the same external I/O before optimistic concurrency rejects one outcome. Reconciliation must repair the durable step chain or atomically exclude mints with active step jobs.🤖 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/lib.rs` around lines 400 - 416, Update spawn_mint_recovery_reconciler and its durable recovery-selection flow so reconciliation cannot enqueue a MintRecoveryJob for a mint already owned by submit, confirm, or callback step jobs. Either repair the durable mint step chain before enqueueing or atomically exclude mints with active step jobs, ensuring recovery and normal mint processing cannot perform duplicate external I/O before optimistic concurrency resolves.
🤖 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.
Inline comments:
In `@src/lib.rs`:
- Around line 1651-1673: The monitor’s clean exit currently breaks the worker
loop and the retrying error path emits repeated WARN logs. In the loop around
monitor.run(), replace the Ok(()) break with the same backoff-and-restart
behavior, log recurring exits/failures at DEBUG or TRACE, and retain
higher-severity logging only for a non-repeating termination path if one exists.
In `@src/mint/job.rs`:
- Around line 634-643: Reorganize the test-module imports so
std::collections::HashMap is grouped with the other external and
standard-library imports, leaving exactly one blank line before the internal
super::* import group.
In `@src/mint/mod.rs`:
- Around line 1092-1119: Update handle_record_tx_submitted to include
Mint::TxIntended in the arm that validates the issuer request and emits
MintTxSubmitted, rather than returning NotInMintingState. Apply the same
TxIntended handling in handle_record_mint_failed, preserving the existing
behavior of each handler’s emitting path.
---
Outside diff comments:
In `@src/jobs/mod.rs`:
- Around line 1-16: Move the interim job runtime from the crate-level jobs
module into the mint feature’s module hierarchy, updating module declarations
and imports so mint::recovery remains the consumer. Revise the module
documentation to remove the stale crate-level/shared-consumer rationale and
describe its mint-scoped interim purpose until event-sorcery provides the
replacement.
In `@src/lib.rs`:
- Around line 400-416: Update spawn_mint_recovery_reconciler and its durable
recovery-selection flow so reconciliation cannot enqueue a MintRecoveryJob for a
mint already owned by submit, confirm, or callback step jobs. Either repair the
durable mint step chain before enqueueing or atomically exclude mints with
active step jobs, ensuring recovery and normal mint processing cannot perform
duplicate external I/O before optimistic concurrency resolves.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88b9c53a-ecd5-426d-8bf7-7e2ebeedac33
📒 Files selected for processing (7)
src/jobs/mod.rssrc/lib.rssrc/mint/api/confirm.rssrc/mint/cmd.rssrc/mint/job.rssrc/mint/mod.rstests/redemption.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: test
- GitHub Check: static
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Package by Feature, Not by Layer - organize by business feature/domain, never by language primitives or technical layers; forbidden module names include types.rs, error.rs, models.rs, utils.rs, helpers.rs, http.rs, dto.rs, entities.rs, services.rs as catch-all technical layers
Zero tolerance for panics in non-test code - forbidden: unwrap(), expect(), panic!(), unreachable!(), unimplemented!() except in test code; forbidden: panicking index operations, division without zero-check; required: use ? for propagation, Result/Option with explicit handling
Keep visibility as restrictive as possible - prefer pub(crate) over pub, private over pub(crate) to enable better dead-code detection and make scope explicit
Use Typed Values in Error types - store typed values, not string representations; forbidden: format!("{value:?}") or .to_string() to convert typed data into error-field strings; correct: store Address, B256, or typed IDs directly
Use #[from] for error variants instead of verbose .map_err() calls - let the compiler guide error variants by using ? as if all required variants exist, then add #[from] variants only for errors the compiler complains about
#[from] variant naming must be generic (mirroring source error type), not claim what operation failed - forbidden: ReadSecret(#[from] io::Error), ParseConfig(#[from] serde_json::Error); correct: Io(#[from] io::Error), Json(#[from] serde_json::Error)
Make invalid states unrepresentable using ADTs/enums to encode business rules - forbidden: types with all/most Option fields, multiple nullable fields that contradict; correct: enum variants for mutually exclusive states with state-specific data inside each variant
Parse, Don't Validate - use newtypes with private inner values and fallible smart constructors as the ONLY way to create constrained domain values; forbidden: separate validate() methods, raw primitives for constrained values, public fields/constructors bypassing validation; applies to API keys, emai...
Files:
src/jobs/mod.rssrc/mint/cmd.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Use two-group import organization: Group 1 (external crates with no blank lines), one blank line separator, Group 2 (internal crate code with no blank lines) - forbidden: three+ groups, blank lines within groups, function-level imports except enum variant imports in function bodies
Never use fully-qualified paths for non-ambiguous types - import them at module top; when ambiguous (e.g. alloy::rpc::types::Log vs alloy::primitives::Log), use qualified imports like rpc_types::Log
Files:
src/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
tests/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.rs: E2E tests must spin up the full HTTP service, use ONLY the public API as external consumer would, use Anvil for local blockchain, mock only truly external systems, assert via API responses and Anvil state; setup phase exception: may use direct SQL to seed event store only, no other tables
E2E tests should test happy paths only and use real blockchain (Anvil)
Files:
tests/redemption.rs
🧠 Learnings (8)
📚 Learning: 2026-02-12T03:36:52.079Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 112
File: src/mint/recovery.rs:78-117
Timestamp: 2026-02-12T03:36:52.079Z
Learning: In Rust recovery loops or similar retry patterns,INFO/WARN log statements that are immediately followed by return; or break; inside the loop body are acceptable because the loop runs at most once for the exit path and won't flood logs. The existing guideline about DEBUG/TRACE logs per iteration applies to repeated logging inside loops; use INFO or WARN for exit paths and avoid per-iteration DEBUG/TRACE logs in retry loops. When reviewing code, flag cases where a log level dominates per-iteration noise and suggest moving exit-logs outside the hot loop or gating by a condition, ensuring the log appears at non-repetitive times.
Applied to files:
src/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
📚 Learning: 2026-02-13T14:15:21.049Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 117
File: src/lib.rs:569-579
Timestamp: 2026-02-13T14:15:21.049Z
Learning: In Rust code, within loops that spawn long-running background tasks (e.g., tokio::spawn) such as detectors, monitors, or services, INFO logs inside the loop body are acceptable if the loop is bounded and each iteration launches a persistent background worker. This acknowledges a significant operational event per iteration without causing excessive log noise. Apply this guidance when the loop has a clear exit condition and the spawned task persists beyond the iteration. If the loop is unbounded or spawns short-lived tasks, prefer lower log levels or structured tracing to avoid log flooding.
Applied to files:
src/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
📚 Learning: 2026-03-19T15:53:49.678Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 130
File: src/config.rs:73-76
Timestamp: 2026-03-19T15:53:49.678Z
Learning: In Rust error enums, follow the AGENTS.md constraint that variant names “must be generic when using #[from]” only for enum variants annotated with #[from] (i.e., variants that derive the `From` conversion for the wrapped error type). For variants populated via manual error construction/handling (e.g., created in code paths using `.map_err(...)`), the naming constraint does not apply—use descriptive or categorical variant names to group related failure reasons (e.g., `HttpClient(Box<dyn Error + Send + Sync>)`).
Applied to files:
src/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
📚 Learning: 2026-06-24T21:02:22.771Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 118
File: src/job.rs:0-0
Timestamp: 2026-06-24T21:02:22.771Z
Learning: In Rust, when implementing the `std::fmt::Display`/`std::Debug` traits (or the core `std::fmt::Formatter`-using `fmt` method), the conventional single-letter parameter name `f` for `fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result` should be allowed. Do not flag `f` as a violation of a “no single-letter variable names” guideline; this name is an established Rust idiom for `Formatter`.
Applied to files:
src/jobs/mod.rssrc/mint/cmd.rstests/redemption.rssrc/mint/job.rssrc/lib.rssrc/mint/mod.rssrc/mint/api/confirm.rs
📚 Learning: 2026-02-17T22:13:31.572Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 122
File: tests/recovery.rs:1013-1139
Timestamp: 2026-02-17T22:13:31.572Z
Learning: In tests located under the tests/ directory, do not include log assertions or tracing-test related checks. Observability assertions (logs) belong in unit or integration tests, not E2E tests. E2E tests should verify public API responses, blockchain state, and external mocks; avoid adding #[traced_test] or logs_contain_at in E2E test files.
Applied to files:
tests/redemption.rs
📚 Learning: 2026-03-20T13:19:14.952Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 131
File: tests/receipt_transfer.rs:156-156
Timestamp: 2026-03-20T13:19:14.952Z
Learning: For ST0x-Technology/st0x.issuance E2E tests under tests/**/*.rs, fixed-sleep waits (tokio::time::sleep with Duration::from_secs) are an accepted pattern to wait for observable conditions. Do not raise an issue for these fixed sleeps as isolated problems. If migrating to a polling/bounded-wait helper (e.g., a shared wait_for utility) is desired, treat it as an intentional cross-cutting refactor that must be applied across all E2E test files simultaneously, rather than changing one file at a time.
Applied to files:
tests/redemption.rs
📚 Learning: 2026-02-17T22:17:07.514Z
Learnt from: 0xgleb
Repo: ST0x-Technology/st0x.issuance PR: 122
File: tests/smoke.rs:1-1
Timestamp: 2026-02-17T22:17:07.514Z
Learning: In test files under tests/**/*.rs, crate-level lint suppression #![allow(clippy::unwrap_used)] is acceptable only when explicitly permitted by the user. E2E tests may benefit from using .unwrap() for immediate panic-on-failure feedback showing the actual value. Apply this guideline only to tests and ensure suppression is clearly documented and restricted to cases approved by the user, avoiding blanket suppression in non-test code.
Applied to files:
tests/redemption.rs
📚 Learning: 2026-06-04T17:02:39.675Z
Learnt from: JuaniRios
Repo: ST0x-Technology/st0x.issuance PR: 168
File: src/lib.rs:993-993
Timestamp: 2026-06-04T17:02:39.675Z
Learning: In `src/lib.rs` and `src/config.rs` for `st0x.issuance`, treat `Config::receipt_poll_interval` as hardcoded from the `RECEIPT_POLL_INTERVAL` constant (60s) set in `Env::into_config`, with no env/CLI override. As long as it remains hardcoded (so `0` is unreachable on production paths), do not flag `tokio::time::interval(config.receipt_poll_interval)` in `spawn_periodic_receipt_backfills` as a potential panic risk. It’s acceptable for tests or external callers to construct `Config` with `receipt_poll_interval = 0` and trigger a panic (use that panic as test feedback). Also, do not introduce a runtime `Config::validate()` method; follow the project rule in `AGENTS.md` (“Parse, Don't Validate”).
Applied to files:
src/lib.rs
🔇 Additional comments (15)
src/jobs/mod.rs (1)
18-149: LGTM!src/lib.rs (2)
3-52: LGTM!Also applies to: 311-311
1616-1650: LGTM!Also applies to: 1674-1739
src/mint/mod.rs (5)
182-201: LGTM!Also applies to: 252-259, 395-414
443-457: LGTM!Also applies to: 510-527
1121-1256: LGTM!
2003-2090: LGTM!Also applies to: 2220-2253
2367-2391: LGTM!Also applies to: 2457-2493
src/mint/cmd.rs (1)
1-8: LGTM!Also applies to: 75-126
src/mint/job.rs (2)
44-52: LGTM!Also applies to: 88-262, 297-349
384-544: LGTM!Also applies to: 564-630
tests/redemption.rs (1)
11-11: LGTM!Also applies to: 212-257, 268-401
src/mint/api/confirm.rs (3)
1-16: LGTM!Also applies to: 33-44, 120-130
137-256: LGTM!
481-575: LGTM!
First commit of RAI-935 (move Mint's side effects into durable jobs). Adds RecordFireblocksSubmitted / RecordTokensMinted / RecordCallbackSent / RecordMintFailed commands + pure, idempotent handlers that emit the existing FireblocksSubmitted / TokensMinted / MintCompleted / MintingFailed events from their payloads with no I/O. Each is a no-op once the mint has advanced past its source state, so an at-least-once job re-run cannot double-record. The durable submit/confirm/callback jobs that dispatch these commands land in follow-up commits.
Adds SubmitMintJob / ConfirmMintJob / SendCallbackJob (drainer-style jobs::Job impls) that perform the Mint aggregate's external side effects off the command handler and report results back via the idempotent outcome commands. Re-runs are double-mint-safe: submit_mint derives a deterministic external_tx_id from the issuer_request_id (Fireblocks dedups), the per-state guards skip work already recorded, and each job enqueues the next. Wired into enqueue + worker registration in the following commit.
process_journal_completion now records Deposit then resolves the vault and enqueues SubmitMintJob; the three drainer workers (submit -> confirm -> callback) drive the mint to completion off the request path. The worker backends use a ~1s fast-poll config (JobQueue::with_fast_poll) so pickup matches the old synchronous flow instead of apalis's 60s idle backoff. The inline SubmitMint/ConfirmMint/SendCallback handlers stay for the recovery path until the recovery rewire.
MintingFailed -> Minting via MintRetryStarted, advancing the automatic-retry attempt counter. Pure, idempotent (no-op once the mint leaves MintingFailed). The recovery driver sends this before re-enqueuing a SubmitMintJob; the deterministic external_tx_id keeps the re-submission double-mint-safe.
Rebase the durable mint job extraction onto jobs-runner, adapt submit to prepare_mint_tx + submit_mint, and wire chain_id through confirm jobs.
Submit/confirm workers were wired to the Base VaultService only, so Ethereum mints called previewDeposit on the wrong chain.
Happy-path mints no longer emit MintTxIntended; roll back to Minting so startup recovery still hits the existing-receipt path.
Accept TxIntended when recording submit/fail so legacy prepares can resume, restart drainer workers on clean monitor exit, and keep job test imports in one group.
Merge activity
|
github-actions
Bot
added
the
externally-merged
3 participants
Motivation
Mint'sSubmitMint/ConfirmMint/SendCallbackcommand handlers ranexternal I/O inline (
vault.submit_mint/vault.confirm_mint+receipts.register_minted_receipt/alpaca.send_mint_callback) plus a DB read.A crash between a vault submission and the event commit is the lost-effect
window ADR-0001 closes — on the most financially-sensitive flow. RAI-935 moves
that I/O into durable apalis jobs that report back idempotent outcome commands,
so the handlers become pure.
Implements RAI-935. This PR covers the normal mint flow. Recovery still
drives stuck mints through the old inline path; rewiring recovery to enqueue
these jobs is a stacked follow-up.
Solution
RecordTxSubmitted/RecordTokensMinted/RecordCallbackSent/RecordMintFailed) — pure, idempotent, a no-op oncetheir event is recorded. Handlers emit the existing events from the command
payload; no event payloads are added or mutated (no permanent-event risk).
src/mint/job.rs):SubmitMintJob→ConfirmMintJob→SendCallbackJob. Each performs one external call, sends itsoutcome command, and enqueues the next step. The resolved vault address and bot
wallet travel in the job context instead of
MintServices.process_journal_completionresolves the vault once and enqueuesSubmitMintJobinstead of sending the old inline-I/O commands. Workers areregistered via
spawn_drainer_worker!.MintingFailedevent (failure-as-eventpreserved); an infrastructure failure surfaces as a job error apalis re-drives.
Re-runs are safe — every outcome command is idempotent against aggregate state,
and
submit_mintderives a deterministicexternal_tx_id.The old inline
SubmitMint/ConfirmMint/SendCallback/Recover/RecoverFromReceiptcommand handlers remain in this PR — recovery still usesthem — and are removed when recovery moves onto the jobs in the follow-up. A pure
RetryMintcommand is included as a building block for that follow-up.Checks
By submitting this for review, I'm confirming I've done the following:
Full workspace suite green — e2e
tests/mint.rsdrives the job chain end-to-end;the redemption e2e tests now wait for the async mint callback before reading
post-mint state. Clippy clean, fmt applied.
Summary by CodeRabbit
New Features
Bug Fixes