feat(minibf): add /assets/{subject}/txs endpoint - #1220
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Mini Blockfrost API adds ChangesAsset transaction lookup
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized API and documentation change has no actionable merge-blocking risk remaining beyond normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant by_subject_txs
participant by_subject_transactions
participant AssetState
participant BlockStore
Client->>by_subject_txs: Request asset transaction hashes
by_subject_txs->>by_subject_transactions: Delegate lookup
by_subject_transactions->>AssetState: Validate asset subject
AssetState-->>by_subject_transactions: Asset state or not found
by_subject_transactions->>BlockStore: Retrieve filtered transactions
BlockStore-->>by_subject_transactions: Paginated transaction models
by_subject_txs-->>Client: Transaction hashes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
michalrus
added
enhancement
There was a problem hiding this comment.
Pull request overview
Adds Blockfrost-compatible support in dolos-minibf for the hash-only asset transactions endpoint, aligning Dolos’ lightweight API surface with Blockfrost clients and expectations around pagination/order semantics.
Changes:
- Add
/assets/{subject}/txsroute that returns only transaction hashes while reusing the existing asset-transaction scan/pagination logic. - Update
/assets/{subject}/transactionsto return404 Not Foundfor valid-but-unknown assets (matching Blockfrost behavior). - Update Minibf API documentation to list the newly supported endpoints (including
/assets/{subject}/txs).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/content/apis/minibf.mdx | Documents the new/now-supported Minibf endpoints, including /assets/{subject}/txs. |
| crates/minibf/src/routes/assets.rs | Implements asset existence check for /transactions, adds /txs handler, and extends test coverage for both endpoints. |
| crates/minibf/src/lib.rs | Registers the new /assets/{subject}/txs route in the Axum router. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
slowbackspace
left a comment
slowbackspace
left a comment
There was a problem hiding this comment.
Reviewed the code and verified behavior against ryo, mimicry, and the official blockfrost-tests suite.
Severity legend: 🔴 must fix · 🟡 should fix · 🟢 optional
Verification done:
- Ran blockfrost-tests on preview with this branch:
assets/:asset/txs5 passed,assets/:asset/transactions9 passed. - Ran blockfrost-tests on mainnet: happy-path, ordering, and from/to tests pass on both endpoints. Six deep-pagination tests fail (
page=42420,page=10000) with themax_scan_items400. The same tests fail on/transactionsbefore this PR. This is inherited, not a regression. - The entity-key hashing matches write-time (
MintStatsUpdate::key). The eager 404 pre-check matches both ryo (assets_404) and mimicry (is_asset_on_chain).
One request for the PR description: this PR changes the contract of the shipped /assets/{subject}/transactions endpoint. A well-formed but unknown asset returned 200 [] before. It now returns 404. The change is correct Blockfrost parity, but please call it out as a behavior change, not only a feature addition.
| let pagination = Pagination::try_from(params)?; | ||
| pagination.enforce_max_scan_limit(domain.config.max_scan_items())?; | ||
|
|
||
| let subject = hex::decode(&subject).map_err(|_| Error::InvalidAsset)?; |
There was a problem hiding this comment.
🟡 Blockfrost validates the asset length before the lookup. validateAsset (ryo) and validate_asset_name (mimicry, src/asset.rs:32) require 56–120 hex chars and return 400 otherwise. Dolos only runs hex::decode. With the new 404 gate, GET /assets/abcd/transactions now returns 404 where Blockfrost returns 400 'Invalid or malformed asset format.' (verified live against this branch). Add a length check that maps to 400. The same gap exists in by_subject and by_subject_addresses; a shared helper would fix all of them.
|
|
||
| let subject = hex::decode(&subject).map_err(|_| Error::InvalidAsset)?; | ||
|
|
||
| // Blockfrost returns 404 for a valid but unknown asset, same as `/addresses`. |
There was a problem hiding this comment.
🟢 The comment cites the wrong precedent. /addresses/{address}/transactions checks existence lazily, only when the page is empty (addresses.rs:550-556), and the /addresses/{address}/txs alias has no check at all. The matching precedent is /assets/{subject}/addresses in this file. Suggest: name that endpoint, or state the Blockfrost rule without a cross-reference.
|
|
||
| // Blockfrost returns 404 for a valid but unknown asset, same as `/addresses`. | ||
| let entity_key = pallas::crypto::hash::Hasher::<256>::hash(subject.as_slice()); | ||
| if !domain.cardano_entity_exists::<AssetState>(entity_key.as_slice())? { |
There was a problem hiding this comment.
🟡 This is the third inline copy of the decode → Hasher::<256> → cardano_entity_exists gate in this file (by_subject line 484, by_subject_addresses line 525). The copies already drift: by_subject maps bad hex to StatusCode::BAD_REQUEST and uses read_cardano_entity. Extract one helper that decodes the subject, validates it, checks existence, and returns the subject bytes. Call it from all three handlers. That also gives the 56–120 length check a single home.
| D: Domain + Clone + Send + Sync + 'static, | ||
| Option<AssetState>: From<D::Entity>, | ||
| { | ||
| let Json(transactions) = by_subject_transactions(path, params, state).await?; |
There was a problem hiding this comment.
🟡 The alias inherits from/to through PaginationParameters, but Blockfrost's /assets/{asset}/txs does not define them. Ryo's handler reads only order/count/page (no getAdditionalParametersFromRequest). Mimicry parses from/to but its assets_asset_txs.sql binds only order, count, page, and asset — the values never reach the query. So both references return the full list for GET /assets/{unit}/txs?from=X&to=Y, while this alias filters by the range. Suggest clearing from/to from the params before delegating.
resolves: #1100
Summary by CodeRabbit
New Features
Bug Fixes
Documentation