Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory_opt calcs wrong memory word range #540

Closed
lightsing opened this issue Jun 14, 2023 · 4 comments
Closed

memory_opt calcs wrong memory word range #540

lightsing opened this issue Jun 14, 2023 · 4 comments

Comments

@lightsing
Copy link
Member

lightsing commented Jun 14, 2023

current we calc the word by this approach:

let start_slot = offset - offset % 32;
let end = offset + length;
let end_slot = end - end % 32;

the range is bytes[start_slot..end_slot+32].

consider we are coping memory of first 32 bytes, aka offset = 0, length = 32.
the words bytes will be bytes[0..63], there comes an extra word.
expect words bytes to be bytes[0..31], only one word.

This should be fixed, with a low priority.

@DreamWuGit @lispc

@DreamWuGit
Copy link
Member

DreamWuGit commented Jun 14, 2023

it is known edge case like in https:/scroll-tech/zkevm-circuits/blob/memory_opt/bus-mapping/src/evm/opcodes/mload.rs#L62, very low priority for now.

@naure
Copy link

naure commented Jun 15, 2023

Can you point to the code to review specifically?

@naure
Copy link

naure commented Jun 23, 2023

This should be fixed here with the function align_range. See also the unittest below. In particular: "When the range ends exactly at a word boundary, it is still ONE WORD."

Please let me know whether we agree on the behavior. (@lightsing @DreamWuGit )

@lightsing
Copy link
Member Author

This should be fixed here with the function align_range. See also the unittest below. In particular: "When the range ends exactly at a word boundary, it is still ONE WORD."

Please let me know whether we agree on the behavior. (@lightsing @DreamWuGit )

I aggree with your fix.

@naure naure closed this as completed Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants