Skip to content

fix(minibf): source address-utxo tx_hash from TxoRef, not archive block_data#1009

Open
adrian1-dot wants to merge 1 commit into
txpipe:mainfrom
adrian1-dot:fix/minibf-address-utxo-txhash
Open

fix(minibf): source address-utxo tx_hash from TxoRef, not archive block_data#1009
adrian1-dot wants to merge 1 commit into
txpipe:mainfrom
adrian1-dot:fix/minibf-address-utxo-txhash

Conversation

@adrian1-dot

@adrian1-dot adrian1-dot commented Jun 2, 2026

Copy link
Copy Markdown

GET /addresses/{address}/utxos returns an empty tx_hash for any UTxO whose source block has aged out of the archive: the handler derives tx_hash from a block_data archive lookup, and on a miss (block_data == None, governed by max_history) .unwrap_or_default() yields "". With the default max_history = 10000 this hits any UTxO older than the window; a Blockfrost client then rejects the 0-length hash.

Fix: source tx_hash from the UTxO's own TxoRef (always present) instead of the archive lookup — identical value when the block is archived, correct value when it isn't.

Repro: set a small [sync] max_history, sync, query an address holding a UTxO created before the window → "tx_hash": "".

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of transaction hash derivation for UTXO outputs by using the transaction reference directly, reducing dependency on external block data sources.

…ck_data

GET /addresses/{address}/utxos returned an empty tx_hash for any UTxO whose
source block aged out of the archive window: tx_hash was built from a
block_data archive lookup, and on a miss (block_data == None, governed by
max_history) unwrap_or_default yields "". Source it from the UTxO's own
TxoRef (always present) instead.
@adrian1-dot adrian1-dot requested a review from scarmuega as a code owner June 2, 2026 20:05
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR changes how AddressUtxoContentInner.tx_hash is derived in the UTxO model builder. Instead of sourcing the hash from block_data with a fallback to empty, the transaction hash is now always derived from the UTxO's own TxoRef. Comments documenting this behavior were updated accordingly.

Changes

UTxO tx_hash derivation

Layer / File(s) Summary
tx_hash source switch to TxoRef
crates/minibf/src/mapping.rs
In UtxoOutputModelBuilder::into_model, the tx_hash field for AddressUtxoContentInner now always derives from self.txo_ref.0.to_string() instead of from block_data with unwrap_or_default(). Inline comments updated to reflect the new source and prior archive-block fallback logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • txpipe/dolos#869: Earlier refactor of UtxoOutputModelBuilder that sourced tx_hash from block_data, which this PR now modifies to derive from TxoRef instead.
  • txpipe/dolos#1006: Overlapping changes to crates/minibf/src/mapping.rs regarding how tx_hash and block metadata are sourced for UTxO model construction.

Suggested reviewers

  • scarmuega

Poem

A UTxO seeks its hash so true,
No more blocks to chase the clue,
TxoRef whispers: "I'm for you!"
Now every tx knows what to do—
One source of truth, the simpler view. 🐰

🚥 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 clearly and specifically describes the main change: sourcing tx_hash from TxoRef instead of archive block_data, which is the core fix addressing the empty tx_hash issue.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/minibf/src/mapping.rs (1)

484-487: ⚡ Quick win

Add a regression test for the archive-miss path.

This mapping change looks correct, but the exact failure mode here was block_data == None, so it should be covered with an assertion that tx_hash still comes from TxoRef and is non-empty when history has aged out. Please also run the required Rust checks for this change before merge. As per coding guidelines, "Run cargo test --workspace --all-features to verify functionality of all changes".

🤖 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 `@crates/minibf/src/mapping.rs` around lines 484 - 487, Add a regression test
that exercises the archive-miss path where block_data == None and asserts that
the produced mapping's tx_hash is taken from the UTxO's TxoRef (i.e., non-empty
and equal to self.txo_ref.0.to_string()); locate the mapping code in mapping.rs
that sets tx_hash and write a test that simulates aged-out history / missing
block_data and verifies tx_hash comes from TxoRef, then run cargo test
--workspace --all-features to validate the change before merging.
🤖 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.

Nitpick comments:
In `@crates/minibf/src/mapping.rs`:
- Around line 484-487: Add a regression test that exercises the archive-miss
path where block_data == None and asserts that the produced mapping's tx_hash is
taken from the UTxO's TxoRef (i.e., non-empty and equal to
self.txo_ref.0.to_string()); locate the mapping code in mapping.rs that sets
tx_hash and write a test that simulates aged-out history / missing block_data
and verifies tx_hash comes from TxoRef, then run cargo test --workspace
--all-features to validate the change before merging.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b1b4f65-bed0-49cf-8a17-0e1e7a867c09

📥 Commits

Reviewing files that changed from the base of the PR and between cc119e9 and 970ce97.

📒 Files selected for processing (1)
  • crates/minibf/src/mapping.rs

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.

1 participant