Skip to content

Conversation

@wetkeyboard
Copy link
Contributor

}
}

batch_start = batch_end + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Integer overflow vulnerability: batch_end + 1 can overflow when batch_end is u64::MAX. This will cause a panic in debug mode or wraparound to 0 in release mode, potentially creating an infinite loop. Fix: Add overflow check before incrementing: if batch_end == u64::MAX { break; } batch_start = batch_end + 1;

Suggested change
batch_start = batch_end + 1;
if batch_end == u64::MAX { break; }
batch_start = batch_end + 1;

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

not opposed to this, but let's try this as a dedicated future type instead

@github-project-automation github-project-automation bot moved this to In Progress in Alloy Sep 16, 2025
@wetkeyboard
Copy link
Contributor Author

Updated, please take another look @mattsse , now the usage like below:

  // Simple log retrieval (unchanged)
  let logs = provider.get_logs(&filter).await?;

  // New enhanced API with batch support
  // Basic usage (equivalent to get_logs)
  let logs = provider.logs(filter).await?;

  // With batching (fetch logs in chunks of 1000 blocks)
  let logs = provider.logs(filter).with_batch_size(1000).await?;

  // With count limit (stop after 500 logs)
  let logs = provider.logs(filter).with_max_count(500).await?;

  // Combined options
  let logs = provider.logs(filter)
      .with_batch_size(1000)
      .with_max_count(500)
      .await?;

@wetkeyboard wetkeyboard requested a review from mattsse September 17, 2025 02:36
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

ty, all of this looks fine

need some time to properly review this

/// The [`EthLogsFut`] future is the future type for batch log retrieval.
#[derive(Debug)]
#[doc(hidden)] // Not public API.
#[expect(unnameable_types)]
Copy link
Contributor

Choose a reason for hiding this comment

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

The attribute should be #[allow(clippy::unnameable_types)] or #[expect(clippy::unnameable_types)] with the full clippy namespace specified. The current form #[expect(unnameable_types)] is incorrect and won't properly suppress the lint warning.

Suggested change
#[expect(unnameable_types)]
#[expect(clippy::unnameable_types)]

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@0xferrous
Copy link
Contributor

can we get this going again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants