Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Fix fetch_previous_block_hashes going out of bounds #12

Merged
merged 1 commit into from
Dec 23, 2023

Conversation

cpubot
Copy link
Contributor

@cpubot cpubot commented Dec 22, 2023

The current implementation attempts to access an out of bounds index when block number is > 255.
In particular, assume block 256 is given.

let start = block_number.saturating_sub(256);
let futs: FuturesOrdered<_> = (start..=block_number)
    .rev()
    .step_by(2)
    .map(|block_number| Self::fetch(rpc_url, block_number))
    .collect();

129 responses are returned.

Later, the responses are iterated over:

for (idx, response) in responses.iter().skip(1).enumerate() {
    // ...
    hashes[starting_offset - 2 * idx] = response.result.hash;
}

Assume starting_offset = 254. When idx reaches 128, starting_offset - 2 * idx == -2.

Assume starting_offset = 253. When idx reaches 127, starting_offset - 2 * idx == -1.

The new implementation provided here constructs the Vec of hashes iteratively, simplifying the logic and avoiding any indexing issues. The solution was tested and verified to be functionally equivalent to the current solution for blocks 0-255.

Copy link
Collaborator

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Good catch, thanks for fixing it!

@cpubot cpubot merged commit efaf274 into main Dec 23, 2023
1 check passed
@cpubot cpubot deleted the zbrown/fix-previous-hash-index-out-of-bounds branch December 23, 2023 16:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants