Skip to content

fix: potential cache issues #7623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SWvheerden wants to merge 1 commit into
base: development
Choose a base branch
from
+10 −2
Open

fix: potential cache issues #7623

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions applications/minotari_node/src/http/handler/sync_utxos_by_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright 2025 The Tari Project
// SPDX-License-Identifier: BSD-3-Clause

use std::{fmt::Display, sync::Arc};
use std::{cmp, fmt::Display, sync::Arc};

use axum::{
extract::Query,
Expand Down Expand Up @@ -87,6 +87,7 @@ pub async fn handle<B: BlockchainBackend + 'static>(
let tip_info = query_service.get_tip_info().await.map_err(error_handler_with_message)?;
let tip_height = tip_info.metadata.map(|m| m.best_block_height()).unwrap_or(0);
let height;
let exclude_spent = request.exclude_spent;
let mut response = match request.version {
0 => {
let response = query_service
Expand All @@ -107,7 +108,14 @@ pub async fn handle<B: BlockchainBackend + 'static>(
body.into_response()
},
};
let last_height = height.unwrap_or(0);
let last_height = if let true = exclude_spent {
// if we're excluding spent outputs we cannot cache for longer than a day, so we force the "height" so that the
// caching will only be a max of 1 day. Wallets will sent full without excluding spends the last 1000 blocks so
// this leaves a margin for wallets to get data if it changed.
Comment on lines +112 to +114
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix comment typo and clarify block count.

The comment has a grammatical error and an inconsistency:

  • "Wallets will sent full" should be "Wallets will send full"
  • The comment mentions "the last 1000 blocks" but the code uses 2500

Please clarify whether 2500 blocks is correct (approximately 1 day) and update the comment accordingly.

🤖 Prompt for AI Agents 
In applications/minotari_node/src/http/handler/sync_utxos_by_block.rs around
lines 112 to 114, fix the typo and clarify the block count in the comment:
change "Wallets will sent full" to "Wallets will send full" and update "the last
1000 blocks" to "the last 2500 blocks (~1 day)" to match the code using 2500 and
explain that 2500 is approximately one day for clarity.

cmp::min(height.unwrap_or(0), tip_height.saturating_add(2500))
} else {
height.unwrap_or(0)
};
Comment on lines +111 to +118
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Dec 4, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the apply_cache_control function and surrounding context
fd -e rs -x grep -l "apply_cache_control" {}

Repository: tari-project/tari

Length of output: 178

🏁 Script executed:

#!/bin/bash
# Search for apply_cache_control implementation
rg -n "fn apply_cache_control" -A 25 -B 2 --type rust

Repository: tari-project/tari

Length of output: 178

🏁 Script executed:

#!/bin/bash
# Search for how tip_height and last_height are used in cache calculations
rg -n "cache.*control|max-age|Cache-Control" -A 5 -B 5 --type rust

Repository: tari-project/tari

Length of output: 178

🏁 Script executed:

#!/bin/bash
# Get the full context of the sync_utxos_by_block.rs file around the flagged section
wc -l applications/minotari_node/src/http/handler/sync_utxos_by_block.rs

Repository: tari-project/tari

Length of output: 178

Fix inverted clamping logic in cache control calculation.

The current code clamps last_height to at most tip_height + 2500:

cmp::min(height.unwrap_or(0), tip_height.saturating_add(2500))

In typical operation where height <= tip_height, this returns height unchanged—providing no cache duration limiting effect.

To limit cache to approximately 1 day when exclude_spent is true, clamp last_height to at least tip_height - 2500:

cmp::max(height.unwrap_or(0), tip_height.saturating_sub(2500))

This ensures old blocks are treated as closer to the tip, reducing cache TTL as intended.

Also simplify the condition: replace if let true = exclude_spent with if exclude_spent.

🤖 Prompt for AI Agents 
In applications/minotari_node/src/http/handler/sync_utxos_by_block.rs around
lines 111 to 118, the cache clamping logic is inverted and the condition is
verbose: change the branch to use `if exclude_spent` (remove `if let true =
...`) and compute last_height by clamping upwards with
cmp::max(height.unwrap_or(0), tip_height.saturating_sub(2500)) so that when
excluding spent outputs old heights are treated as at least tip_height - 2500
(reducing cache TTL as intended).


apply_cache_control(
response.headers_mut(),
Expand Down
Loading
⚡