Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

fix finding 24 #656

Closed
wants to merge 1 commit into from
Closed

Conversation

kunxian-xia
Copy link

@kunxian-xia kunxian-xia commented Jul 19, 2023

Description

This PR aims to fix finding 24 in Zellic / Kalos Audit Report.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Copy link

@roynalnaruto roynalnaruto left a comment

Choose a reason for hiding this comment

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

Is the byte check for each diff cell required? As its already done in the LtGadget

vec![(q_enable * cum_num_txs, u16_table)]
});

for diff in tx_id_cmp_cum_num_txs.lt_chip.config().diff {

Choose a reason for hiding this comment

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

We want the overall diff to be a byte, right?

Here you've constrained that each diff cell is a byte, which is already constrained within the LtGadget here:
https://github.com/scroll-tech/zkevm-circuits/blob/develop/gadgets/src/less_than.rs#L97-L105

Copy link
Author

Choose a reason for hiding this comment

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

We add this because the range checks of each diff cell are not there when the auditing is done. Yeah, we should remove these redundant checks.

@kunxian-xia kunxian-xia closed this Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants