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

Add bounds for out of range check in get_block_hash syscall #1167

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

toni-calvin
Copy link
Contributor

@toni-calvin toni-calvin commented Dec 4, 2023

TITLE

Description

This PR fixes #1166

Description of the pull request changes and motivation.

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.

self.handle_syscall_request(gas, "get_block_hash")?;

let current_block_number = dbg!(self.block_context.block_info.block_number);
if block_number > current_block_number - 10 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This operation can panick, lets check that current_block_number is >= 10

Copy link
Contributor

Choose a reason for hiding this comment

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

Or you can do this instead:

Suggested change
if block_number > current_block_number - 10 {
if block_number > current_block_number.saturating_sub(10) {

Copy link
Contributor

Choose a reason for hiding this comment

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

Or compare with an addition instead:

Suggested change
if block_number > current_block_number - 10 {
if block_number + 10 > current_block_number {

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2023

Codecov Report

Merging #1167 (48bf482) into main (abeefe7) will decrease coverage by 0.15%.
The diff coverage is 0.00%.

❗ Current head 48bf482 differs from pull request most recent head 8f91882. Consider uploading reports for the commit 8f91882 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1167      +/-   ##
==========================================
- Coverage   89.61%   89.47%   -0.15%     
==========================================
  Files          50       50              
  Lines       14370    14369       -1     
==========================================
- Hits        12878    12856      -22     
- Misses       1492     1513      +21     
Files Coverage Δ
src/syscalls/business_logic_syscall_handler.rs 78.81% <0.00%> (ø)

... and 3 files with indirect coverage changes

@toni-calvin toni-calvin force-pushed the fix-get-block-hash-bounds branch from 48bf482 to 8f91882 Compare December 4, 2023 20:01
Copy link
Contributor

@fmoletta fmoletta left a comment

Choose a reason for hiding this comment

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

🚀

@fmoletta fmoletta added this pull request to the merge queue Dec 4, 2023
Merged via the queue into main with commit c2cf08b Dec 4, 2023
7 checks passed
@fmoletta fmoletta deleted the fix-get-block-hash-bounds branch December 4, 2023 21:43
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.

Incorrect bounds for out of range check in get_block_hash syscall
4 participants