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

Fix RLC is_lt_128 #1449

Merged
merged 4 commits into from
Jun 2, 2023
Merged

Fix RLC is_lt_128 #1449

merged 4 commits into from
Jun 2, 2023

Conversation

naure
Copy link
Contributor

@naure naure commented Jun 1, 2023

Description

In the RLC gadget, the indicator is_lt_128 must be equivalent to value < 128.

However, currently only the true case is handled. This PR handles both true and false cases.

Issue Link

Scroll internal.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@github-actions github-actions bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Jun 1, 2023
han0110
han0110 approved these changes Jun 2, 2023
View reviewed changes
Copy link
Collaborator

@han0110 han0110 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small nitpick.

zkevm-circuits/src/evm_circuit/util/math_gadget/rlp.rs Outdated Show resolved Hide resolved
ed255
ed255 approved these changes Jun 2, 2023
View reviewed changes
Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! I agree on the redundant constraint pointed out by Han.

@naure @han0110
Remove redundant bool check
8a7d42d 
Co-authored-by: Han <tinghan0110@gmail.com>
@ed255 ed255 added this pull request to the merge queue Jun 2, 2023
Merged via the queue into privacy-scaling-explorations:main with commit 7b77f9c Jun 2, 2023
@naure naure deleted the TOB-SCROLL-2 branch June 2, 2023 17:19
@naure naure mentioned this pull request Jul 11, 2023
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@han0110 han0110 han0110 approved these changes

@ed255 ed255 ed255 approved these changes

Assignees
No one assigned
Labels
crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@naure @han0110 @ed255