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

Bug: fix to truncate offset from Word to Uint64 when length is zero for some opcodes#1415

Merged
ChihChengLiang merged 9 commits into
privacy-ethereum:mainfrom
scroll-tech:bug/truncate-word-to-u64-for-zero-len
May 20, 2023
Merged

Bug: fix to truncate offset from Word to Uint64 when length is zero for some opcodes#1415
ChihChengLiang merged 9 commits into
privacy-ethereum:mainfrom
scroll-tech:bug/truncate-word-to-u64-for-zero-len

Conversation

@silathdiir
Copy link
Copy Markdown
Contributor

@silathdiir silathdiir commented May 17, 2023

Description

Reference go-ethereum function calcMemSize64WithUint, ignore to check Uint64 overflow for memory offset if length is zero.

it is also called by calcMemSize64. And both are used for opcodes in memory_table.go as memorySize in jump_table.go.

For Call opcodes, in offset and size are truncated to Uint64 as opCall.

Issue Link

Related issue #1301

Original local PR scroll-tech#393

Type of change

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

How Has This Been Tested?

  1. Fix testool case randomStatetest85_d0_g0_v0.
  2. Add test cases of overflow offset and zero length for related opcodes.

@github-actions github-actions Bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels May 17, 2023
@ChihChengLiang ChihChengLiang self-requested a review May 17, 2023 14:22
@hero78119 hero78119 mentioned this pull request May 17, 2023
8 tasks
@silathdiir silathdiir force-pushed the bug/truncate-word-to-u64-for-zero-len branch 2 times, most recently from a6ea6d6 to 5eb2f24 Compare May 18, 2023 07:20
@ChihChengLiang ChihChengLiang requested a review from lispc May 19, 2023 05:34
Copy link
Copy Markdown
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the fix and adding tests.

@ChihChengLiang ChihChengLiang enabled auto-merge May 19, 2023 07:53
@ChihChengLiang ChihChengLiang disabled auto-merge May 19, 2023 13:21
@ChihChengLiang ChihChengLiang enabled auto-merge May 19, 2023 13:25
@ChihChengLiang
Copy link
Copy Markdown
Collaborator

@silathdiir a rebase to main to apply the CI fix is required to merge this.

auto-merge was automatically disabled May 19, 2023 13:33

Head branch was pushed to by a user without write access

@silathdiir silathdiir force-pushed the bug/truncate-word-to-u64-for-zero-len branch from 0100bf1 to 03f1a14 Compare May 19, 2023 13:33
@silathdiir silathdiir force-pushed the bug/truncate-word-to-u64-for-zero-len branch from 03f1a14 to 9030455 Compare May 19, 2023 14:09
@ChihChengLiang ChihChengLiang added this pull request to the merge queue May 20, 2023
Merged via the queue into privacy-ethereum:main with commit 43c67c9 May 20, 2023
@silathdiir silathdiir deleted the bug/truncate-word-to-u64-for-zero-len branch May 20, 2023 07:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants