Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end script support: emits SCRIPT tags in index deltas, extracts script hashes from outputs and witnesses during batch processing, provides a query to find scripts by hash (language + bytes) and exposes new HTTP endpoints returning script/datum JSON and CBOR. Changes
Sequence DiagramsequenceDiagram
participant BP as BlockProcessor
participant DB as DeltaBuilder
participant AR as ArchiveIndex
participant QF as QueryFacade
participant HTTP as HTTPHandler
BP->>BP: process block & transactions
alt outputs or witnesses contain scripts / script_ref
BP->>BP: extract script bytes & determine language
BP->>DB: add_script_hash(hash)
DB->>AR: append SCRIPT tag to block
end
HTTP->>QF: script_by_hash(hash) request
QF->>AR: iterate blocks by tag=SCRIPT up to tip
AR-->>QF: SCRIPT-tagged blocks
QF->>QF: inspect native/plutus scripts & script_refs
alt match found
QF-->>HTTP: return ScriptData(language, script bytes)
else
QF-->>HTTP: return None
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Open
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/cardano/src/roll/batch.rs (1)
427-440: Remove unnecessary inner block scope.The
{}wrapping the witness-script loops adds a level of indentation without any scoping benefit — no variables are declared that need to be dropped or shadowed.♻️ Proposed refactor
- // Witness scripts - { - for script in tx.native_scripts() { - builder.add_script_hash(script.original_hash().to_vec()); - } - for script in tx.plutus_v1_scripts() { - builder.add_script_hash(script.compute_hash().to_vec()); - } - for script in tx.plutus_v2_scripts() { - builder.add_script_hash(script.compute_hash().to_vec()); - } - for script in tx.plutus_v3_scripts() { - builder.add_script_hash(script.compute_hash().to_vec()); - } - } + // Witness scripts + for script in tx.native_scripts() { + builder.add_script_hash(script.original_hash().to_vec()); + } + for script in tx.plutus_v1_scripts() { + builder.add_script_hash(script.compute_hash().to_vec()); + } + for script in tx.plutus_v2_scripts() { + builder.add_script_hash(script.compute_hash().to_vec()); + } + for script in tx.plutus_v3_scripts() { + builder.add_script_hash(script.compute_hash().to_vec()); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cardano/src/roll/batch.rs` around lines 427 - 440, Remove the unnecessary inner block that wraps the witness-script loops: delete the surrounding `{ ... }` so the four for-loops (iterating tx.native_scripts(), tx.plutus_v1_scripts(), tx.plutus_v2_scripts(), tx.plutus_v3_scripts() and calling builder.add_script_hash(...)) live directly in the parent scope without the extra braces; no other logic changes are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cardano/src/roll/batch.rs`:
- Around line 408-440: The code currently may append duplicate script hashes
because both output.script_ref() handling and witness script loops call
add_script_hash unconditionally; remove the extra surrounding block scope and
deduplicate by collecting script hashes into a HashSet first (use the
script.original_hash()/compute_hash() values from output.script_ref(),
tx.native_scripts(), tx.plutus_v1_scripts(), tx.plutus_v2_scripts(),
tx.plutus_v3_scripts()), then iterate the set and call
builder.add_script_hash(...) once per unique hash; ensure you reference the
existing functions/methods (output.script_ref(),
ScriptRef::{NativeScript,PlutusV1Script,PlutusV2Script,PlutusV3Script},
tx.native_scripts(), tx.plutus_v1_scripts(), tx.plutus_v2_scripts(),
tx.plutus_v3_scripts(), and builder.add_script_hash) when implementing the
change.
---
Nitpick comments:
In `@crates/cardano/src/roll/batch.rs`:
- Around line 427-440: Remove the unnecessary inner block that wraps the
witness-script loops: delete the surrounding `{ ... }` so the four for-loops
(iterating tx.native_scripts(), tx.plutus_v1_scripts(), tx.plutus_v2_scripts(),
tx.plutus_v3_scripts() and calling builder.add_script_hash(...)) live directly
in the parent scope without the extra braces; no other logic changes are
required.
Comment on lines
+408
to
440
| if let Some(script_ref) = output.script_ref() { | ||
| match script_ref { | ||
| ScriptRef::NativeScript(script) => { | ||
| builder.add_script_hash(script.original_hash().to_vec()); | ||
| } | ||
| ScriptRef::PlutusV1Script(script) => { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| ScriptRef::PlutusV2Script(script) => { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| ScriptRef::PlutusV3Script(script) => { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Witness scripts | ||
| { | ||
| for script in tx.native_scripts() { | ||
| builder.add_script_hash(script.original_hash().to_vec()); | ||
| } | ||
| for script in tx.plutus_v1_scripts() { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| for script in tx.plutus_v2_scripts() { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| for script in tx.plutus_v3_scripts() { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find where add_script_hash is defined
fd -t f "delta.rs" | head -5Repository: txpipe/dolos
Length of output: 91
🏁 Script executed:
# Search for add_script_hash method definition
rg "fn add_script_hash" --type rs -A 10Repository: txpipe/dolos
Length of output: 82
🏁 Script executed:
# Look for the builder implementation and SCRIPT tag handling
rg "add_script_hash|SCRIPT" crates/cardano/src/roll/ --type rs -B 2 -A 2Repository: txpipe/dolos
Length of output: 82
🏁 Script executed:
# Check if there's any deduplication logic in the codebase
rg "deduplicate|dedup|HashSet|BTreeSet" crates/cardano/src/roll/ --type rs | head -20Repository: txpipe/dolos
Length of output: 82
🏁 Script executed:
# Examine the full context of the batch.rs file around those lines
sed -n '1,30p' crates/cardano/src/roll/batch.rsRepository: txpipe/dolos
Length of output: 1109
🏁 Script executed:
# Check the actual implementation of add_script_hash and surrounding code
sed -n '400,450p' crates/cardano/src/roll/batch.rsRepository: txpipe/dolos
Length of output: 2327
🏁 Script executed:
# Search for add_script_hash implementation in delta.rs
rg "fn add_script_hash" crates/cardano/src/indexes/delta.rs -A 10Repository: txpipe/dolos
Length of output: 490
🏁 Script executed:
# Look for the builder type and how SCRIPT tags are handled
rg "add_script_hash|SCRIPT" crates/cardano/src/indexes/delta.rs -B 2 -A 5 | head -50Repository: txpipe/dolos
Length of output: 448
🏁 Script executed:
# Check the data structure used to store script hashes (sets, vectors, etc.)
rg "struct.*Builder|field.*script" crates/cardano/src/indexes/delta.rs -A 15 | head -100Repository: txpipe/dolos
Length of output: 446
🏁 Script executed:
# Look for deduplication or set usage in the entire indexes module
rg "HashSet|BTreeSet|dedup" crates/cardano/src/indexes/ --type rsRepository: txpipe/dolos
Length of output: 82
🏁 Script executed:
# Check if add_script_hash is called elsewhere and how duplicates might be handled
rg "add_script_hash" crates/ --type rsRepository: txpipe/dolos
Length of output: 82
🏁 Script executed:
# Search for IndexDelta structure definition
rg "struct IndexDelta" crates/ -A 15Repository: txpipe/dolos
Length of output: 955
🏁 Script executed:
# Look for where IndexDelta is used and if tags are processed/deduplicated
rg "IndexDelta" crates/cardano/src/ -B 2 -A 3 | head -50Repository: txpipe/dolos
Length of output: 2762
🏁 Script executed:
# Check if there's any post-processing of tags before storage
rg "\.tags" crates/ --type rust | head -30Repository: txpipe/dolos
Length of output: 2480
🏁 Script executed:
# Look for script handling tests or documentation
rg "script_ref|add_script_hash" crates/ --type rust -B 3 -A 3 | head -80Repository: txpipe/dolos
Length of output: 5297
🏁 Script executed:
# Search for any existing duplicate handling patterns in the codebase
rg "Vec.*clone|to_vec" crates/cardano/src/roll/batch.rs -B 2 -A 2 | head -50Repository: txpipe/dolos
Length of output: 2192
🏁 Script executed:
# Check if there's any test coverage or comments about duplicate script handling
rg "script.*duplicate|duplicate.*script" crates/ -iRepository: txpipe/dolos
Length of output: 38
🏁 Script executed:
# Look for any deduplication in the tag processing at storage layer
rg "apply.*tags|history.*tags|utxos.*tags" crates/ --type rust -B 3 -A 5 | head -80Repository: txpipe/dolos
Length of output: 3025
🏁 Script executed:
# Examine how tags are applied/stored in the index
cat crates/fjall/src/index/utxos.rs | head -100Repository: txpipe/dolos
Length of output: 3181
🏁 Script executed:
# Check the archive block structure more thoroughly
rg "struct.*Block|struct.*ArchiveIndexDelta" crates/ -A 10 | head -80Repository: txpipe/dolos
Length of output: 4636
Remove duplicate script-hash indexing and unnecessary block scope.
The output script_ref loop (lines 408–424) and witness scripts loop (lines 428–439) both unconditionally call add_script_hash within the same transaction. A transaction that produces a reference script and uses that same script in its witnesses will push the same hash twice to the block's tag list. Since add_script_hash simply appends to a vector without deduplication, this creates duplicate SCRIPT tags.
Additionally, the {} wrapper around the witness scripts block (lines 427–440) is unnecessary and only adds indentation.
Consider deduplicating script hashes before indexing, or skip witness scripts that were already added via script_ref. The block scope can be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cardano/src/roll/batch.rs` around lines 408 - 440, The code currently
may append duplicate script hashes because both output.script_ref() handling and
witness script loops call add_script_hash unconditionally; remove the extra
surrounding block scope and deduplicate by collecting script hashes into a
HashSet first (use the script.original_hash()/compute_hash() values from
output.script_ref(), tx.native_scripts(), tx.plutus_v1_scripts(),
tx.plutus_v2_scripts(), tx.plutus_v3_scripts()), then iterate the set and call
builder.add_script_hash(...) once per unique hash; ensure you reference the
existing functions/methods (output.script_ref(),
ScriptRef::{NativeScript,PlutusV1Script,PlutusV2Script,PlutusV3Script},
tx.native_scripts(), tx.plutus_v1_scripts(), tx.plutus_v2_scripts(),
tx.plutus_v3_scripts(), and builder.add_script_hash) when implementing the
change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/minibf/src/routes/scripts.rs (1)
26-46: Optional: deduplicate hash parsing helpers.
parse_script_hashandparse_datum_hashare structurally identical except for size. A small generic helper would reduce repetition and keep error behavior centralized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/minibf/src/routes/scripts.rs` around lines 26 - 46, The two functions parse_script_hash and parse_datum_hash are identical except for the hash size; replace them with a single generic helper like parse_hash<const N: usize>(s: &str) -> Result<Hash<N>, StatusCode> that validates length (2*N bytes => hex length 2*N), decodes hex with hex::decode and maps errors to StatusCode::NOT_FOUND, then constructs Hash::<N>::from(decoded.as_slice()); update parse_script_hash and parse_datum_hash to call this generic helper (or remove them and call parse_hash::<28> and parse_hash::<32> where used) so all parsing and error mapping is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/minibf/src/routes/scripts.rs`:
- Around line 151-159: The handler currently treats the Option returned by
domain.query().plutus_data(&parse_datum_hash(&datum_hash)?) as if it were always
Some, which can yield a 200 for missing data; update the logic in the function
that returns Json(ScriptDatumCbor) so that if datum is None you return an
Err(StatusCode::NOT_FOUND) (similar to by_datum_hash), otherwise serialize the
datum with minicbor::to_vec and hex::encode as before; reference the
domain.query().plutus_data call, parse_datum_hash(&datum_hash), ScriptDatumCbor
construction, and use StatusCode::NOT_FOUND for the missing case while keeping
existing map_err usages for internal errors.
---
Nitpick comments:
In `@crates/minibf/src/routes/scripts.rs`:
- Around line 26-46: The two functions parse_script_hash and parse_datum_hash
are identical except for the hash size; replace them with a single generic
helper like parse_hash<const N: usize>(s: &str) -> Result<Hash<N>, StatusCode>
that validates length (2*N bytes => hex length 2*N), decodes hex with
hex::decode and maps errors to StatusCode::NOT_FOUND, then constructs
Hash::<N>::from(decoded.as_slice()); update parse_script_hash and
parse_datum_hash to call this generic helper (or remove them and call
parse_hash::<28> and parse_hash::<32> where used) so all parsing and error
mapping is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 45730ade-b307-4198-a12a-e3fb65ec671f
📒 Files selected for processing (4)
crates/minibf/src/lib.rscrates/minibf/src/mapping.rscrates/minibf/src/routes/scripts.rsdocs/content/apis/minibf.mdx
scarmuega
approved these changes
Mar 4, 2026
scarmuega
reviewed
Mar 4, 2026
| builder.add_script_hash(script.original_hash().to_vec()); | ||
| } | ||
| ScriptRef::PlutusV1Script(script) => { | ||
| builder.add_script_hash(script.compute_hash().to_vec()); |
scarmuega
reviewed
Mar 4, 2026
crates/minibf/src/routes/scripts.rs
Outdated
| })) | ||
| } | ||
|
|
||
| fn native_script_json(script: &NativeScript) -> Result<serde_json::Value, StatusCode> { |
Member
There was a problem hiding this comment.
IIRC, there's a trait called ToCanonicalJson that provides this serialization out-of-the-box.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/minibf/src/routes/scripts.rs (1)
94-95: Remove commented-out code.Line 94 contains dead code that should be removed to maintain code cleanliness.
♻️ Proposed fix
let native: NativeScript = minicbor::decode(&script.script).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - // Some(native_script_json(&native)?) Some(native.to_json())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/minibf/src/routes/scripts.rs` around lines 94 - 95, Remove the dead commented-out code "// Some(native_script_json(&native)?)" in crates/minibf/src/routes/scripts.rs to keep the codebase clean; specifically delete that commented line near the Some(native.to_json()) return so only Some(native.to_json()) remains (references: native_script_json and native.to_json) and ensure no other commented legacy call remains in the surrounding function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/minibf/src/routes/scripts.rs`:
- Around line 94-95: Remove the dead commented-out code "//
Some(native_script_json(&native)?)" in crates/minibf/src/routes/scripts.rs to
keep the codebase clean; specifically delete that commented line near the
Some(native.to_json()) return so only Some(native.to_json()) remains
(references: native_script_json and native.to_json) and ensure no other
commented legacy call remains in the surrounding function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6054b2f-2a11-4cf3-893a-748dde6fa2b9
📒 Files selected for processing (4)
crates/minibf/src/lib.rscrates/minibf/src/mapping.rscrates/minibf/src/routes/scripts.rsdocs/content/apis/minibf.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/minibf/src/lib.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/minibf/src/routes/scripts.rs (1)
94-95: Remove commented-out code.Line 94 contains dead code that should be removed to keep the codebase clean.
🧹 Proposed cleanup
let native: NativeScript = minicbor::decode(&script.script).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - // Some(native_script_json(&native)?) Some(native.to_json())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/minibf/src/routes/scripts.rs` around lines 94 - 95, Remove the dead commented-out invocation native_script_json(&native)? from crates/minibf/src/routes/scripts.rs and leave only the active return Some(native.to_json()); — delete the commented line referencing native_script_json to clean up the codebase and avoid stale code, ensuring the surrounding function (the match arm returning Some(...)) and the native.to_json() call remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/minibf/src/routes/scripts.rs`:
- Around line 94-95: Remove the dead commented-out invocation
native_script_json(&native)? from crates/minibf/src/routes/scripts.rs and leave
only the active return Some(native.to_json()); — delete the commented line
referencing native_script_json to clean up the codebase and avoid stale code,
ensuring the surrounding function (the match arm returning Some(...)) and the
native.to_json() call remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da7e8615-b468-40f0-9805-013f7df1383a
📒 Files selected for processing (1)
crates/minibf/src/routes/scripts.rs
scarmuega
approved these changes
Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Indexing
Data/Serialization
Documentation