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

fix/keccak-decode-overflow: calculate with bitshifts in u64#1262

Merged
CPerezz merged 1 commit into
privacy-ethereum:mainfrom
scroll-tech:pse-fix/keccak-decode-overflow
Feb 27, 2023
Merged

fix/keccak-decode-overflow: calculate with bitshifts in u64#1262
CPerezz merged 1 commit into
privacy-ethereum:mainfrom
scroll-tech:pse-fix/keccak-decode-overflow

Conversation

@naure

@naure naure commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

Description

With KECCAK_DEGREE=19, get_num_bits_per_absorb_lookup() is 11. In some places, 2**3**11 overflows u32.

Replace with u64. Also use faster bitshifts like Axiom’s version.

Issue Link

Port fix from Scroll scroll-tech#335

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

Contents

  • Compute in u64

How Has This Been Tested?

cargo test -p zkevm-circuits --features test --release -- keccak

@github-actions github-actions Bot added the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Feb 27, 2023

@CPerezz CPerezz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Thanks for porting this here!

@CPerezz CPerezz added this pull request to the merge queue Feb 27, 2023
Merged via the queue into privacy-ethereum:main with commit 35c4f5c Feb 27, 2023
@naure naure deleted the pse-fix/keccak-decode-overflow branch February 27, 2023 15:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

crate-zkevm-circuits Issues related to the zkevm-circuits workspace member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants