-
Notifications
You must be signed in to change notification settings - Fork 182
fix: convert look back limit to actual epoch before searching for msg #6354
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1046,17 +1046,17 @@ where | |
| &self, | ||
| mut current: Tipset, | ||
| message: &ChainMessage, | ||
| look_back_limit: Option<i64>, | ||
| allow_replaced: Option<bool>, | ||
| lookback_max_epoch: ChainEpoch, | ||
| allow_replaced: bool, | ||
| ) -> Result<Option<(Tipset, Receipt)>, Error> { | ||
| let allow_replaced = allow_replaced.unwrap_or(true); | ||
| let message_from_address = message.from(); | ||
| let message_sequence = message.sequence(); | ||
| let mut current_actor_state = self | ||
| .get_required_actor(&message_from_address, *current.parent_state()) | ||
| .map_err(Error::state)?; | ||
| let message_from_id = self.lookup_required_id(&message_from_address, ¤t)?; | ||
| while current.epoch() > look_back_limit.unwrap_or_default() { | ||
|
|
||
| while current.epoch() >= lookback_max_epoch { | ||
| let parent_tipset = self | ||
| .chain_index() | ||
| .load_required_tipset(current.parents()) | ||
|
|
@@ -1091,14 +1091,29 @@ where | |
| Ok(None) | ||
| } | ||
|
|
||
| /// Searches backwards through the chain for a message receipt. | ||
| fn search_back_for_message( | ||
| &self, | ||
| current: Tipset, | ||
| message: &ChainMessage, | ||
| look_back_limit: Option<i64>, | ||
| allow_replaced: Option<bool>, | ||
| ) -> Result<Option<(Tipset, Receipt)>, Error> { | ||
| self.check_search(current, message, look_back_limit, allow_replaced) | ||
| let current_epoch = current.epoch(); | ||
| let allow_replaced = allow_replaced.unwrap_or(true); | ||
|
|
||
| // Calculate the max lookback epoch (inclusive lower bound) for the search. | ||
| let lookback_max_epoch = match look_back_limit { | ||
| // No search: limit = 0 means search 0 epochs | ||
| Some(0) => return Ok(None), | ||
| // Limited search: calculate the inclusive lower bound | ||
| // For ex: limit = 5 at epoch 1000: min_epoch = 996, searches [996, 1000] = 5 epochs | ||
| Some(limit) if limit > 0 && limit < current_epoch => current_epoch - limit + 1, | ||
| // Search all the way to genesis (epoch 0) | ||
| _ => 0, | ||
| }; | ||
|
Comment on lines
+1105
to
+1114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential off-by-one error when limit equals current_epoch. The condition Example at epoch 1000:
🔧 Suggested fix- // Calculate the max lookback epoch (inclusive lower bound) for the search.
+ // Calculate the lookback epoch (inclusive lower bound) for the search.
let lookback_max_epoch = match look_back_limit {
// No search: limit = 0 means search 0 epochs
Some(0) => return Ok(None),
- // Limited search: calculate the inclusive lower bound
- // For ex: limit = 5 at epoch 1000: min_epoch = 996, searches [996, 1000] = 5 epochs
- Some(limit) if limit > 0 && limit < current_epoch => current_epoch - limit + 1,
- // Search all the way to genesis (epoch 0)
+ // Limited search: calculate the inclusive lower bound, clamped to genesis
+ // Example: limit=5 at epoch=1000 → min_epoch=996, searches [996,1000] = 5 epochs
+ // Example: limit=2000 at epoch=1000 → min_epoch=0, searches [0,1000] = 1001 epochs (all available)
+ Some(limit) if limit > 0 => (current_epoch - limit + 1).max(0),
+ // Unlimited search: None or negative limit (e.g., -1) searches to genesis (epoch 0)
_ => 0,
};This change:
🤖 Prompt for AI Agents |
||
|
|
||
| self.check_search(current, message, lookback_max_epoch, allow_replaced) | ||
| } | ||
|
|
||
| /// Returns a message receipt from a given tipset and message CID. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's document this constant while we're at it; the name is not self-explanatory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.