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

Bug: fix to truncate memory offset of Word to Uint64 when memory length is zero#393

Merged
lispc merged 8 commits into
scroll-stablefrom
bug/truncate-word-to-u64-for-zero-len
Mar 10, 2023
Merged

Bug: fix to truncate memory offset of Word to Uint64 when memory length is zero#393
lispc merged 8 commits into
scroll-stablefrom
bug/truncate-word-to-u64-for-zero-len

Conversation

@silathdiir
Copy link
Copy Markdown

@silathdiir silathdiir commented Mar 9, 2023

Description

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

it is also called by calcMemSize64. And both are widely used in memory_table.go (as memorySize in jump_table.go).

For CALLCODE, memory offset is truncate to Uint64 directly in opCallCode.

Review and fix the all related opcodes in memory_table.go.

Issue Link

Related upstream issue privacy-ethereum#1301

TODO

Should we also need to constrain memory length is zero when memory offset is overflow in circuit?

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 memory offset and zero length for related opcodes.

@silathdiir silathdiir requested a review from lispc March 9, 2023 14:20
@silathdiir silathdiir marked this pull request as draft March 10, 2023 00:45
@silathdiir silathdiir force-pushed the bug/truncate-word-to-u64-for-zero-len branch from d841582 to efe782d Compare March 10, 2023 00:57
@silathdiir silathdiir changed the title Bug: fix to truncate memory offset of Word to Uint64 for CALL opcodes Bug: fix to truncate memory offset of Word to Uint64 when memory length is zero Mar 10, 2023
@silathdiir silathdiir force-pushed the bug/truncate-word-to-u64-for-zero-len branch from efe782d to bd5e6d9 Compare March 10, 2023 01:47
@silathdiir silathdiir marked this pull request as ready for review March 10, 2023 09:17
@lispc lispc merged commit 095bc5b into scroll-stable Mar 10, 2023
@lispc lispc deleted the bug/truncate-word-to-u64-for-zero-len branch March 10, 2023 11:40
@lispc
Copy link
Copy Markdown

lispc commented Mar 11, 2023

need to apply this when PR to upstream 598342b @silathdiir

@silathdiir silathdiir restored the bug/truncate-word-to-u64-for-zero-len branch March 13, 2023 05:52
github-merge-queue Bot pushed a commit to privacy-ethereum/zkevm-circuits that referenced this pull request Apr 28, 2023
#1317)

### Description

Fix successful run cases with Uint64 overflow for multiple opcodes.

1. Add `WordByteRangeGadget` to constrain if Word is within the
specified byte range.

2. Add `WordByteCapGadget` to constrain if Word is within the specified
byte range (implemented by WordByteRangeGadget) and less than a maximum
cap (used to replace a WordByteRangeGadget and LtGadget).

3. Fix bus-mapping and zkevm-circuits to handle overflow cases. And add
unit-tests for these cases.

TODO: will try to handle memory offset overflow with zero length in
another PR (try to rebase for this local PR
scroll-tech#393) and related
issue
#1301.

### Rationale

Reference detailed code in `go-etherum` as:

.
[BLOCKHASH](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L438)
.
[CALLDATALOAD](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L285)
.
[CALLDATACOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L306)
.
[CODECOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L364)
.
[EXTCODECOPY](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L382)
.
[JUMPI](https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L550)

### Issue Link

Close
#1276

### Type of change

- [X] Bug fix (non-breaking change which fixes an issue)

### How Has This Been Tested?

Add unit-test cases for Uint64 overflow values.

---------

Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
github-merge-queue Bot pushed a commit to privacy-ethereum/zkevm-circuits that referenced this pull request May 20, 2023
…or some opcodes (#1415)

### Description

Reference go-ethereum function
[calcMemSize64WithUint](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/common.go#L38),
ignore to check Uint64 overflow for memory offset if length is zero.

it is also called by
[calcMemSize64](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/common.go#LL31C9-L31C30).
And both are used for opcodes in
[memory_table.go](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/memory_table.go#L20)
as `memorySize` in
[jump_table.go](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/jump_table.go#L387).

For Call opcodes, in offset and size are truncated to Uint64 as
[opCall](https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/instructions.go#LL672C60-L672C60).

### Issue Link

Related issue
#1301

Original local PR scroll-tech#393

### Type of change

- [X] 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.
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