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

FIP-0094: Add Support for EIP-5656 (MCOPY Opcode) in the FEVM #1047

Merged
merged 6 commits into from
Sep 2, 2024

Conversation

snissn
Copy link
Contributor

@snissn snissn commented Aug 22, 2024

The MCOPY opcode provides a more efficient mechanism for memory copying within smart contracts. This proposal suggests integrating MCOPY into the FEVM, ensuring compatibility with Solidity versions v0.8.25 and later, which rely on this opcode for certain operations.

#1025

A series of test cases will be provided to ensure that the `MCOPY` opcode functions as expected under various scenarios. These test cases will include non-overlapping memory copy, overlapping memory copy, out-of-bounds source handling, and memory expansion.

## Security Considerations
Security implications for the `MCOPY` opcode are similar to other memory manipulation operations. Special care will be taken to ensure that overlapping memory regions are handled safely, preventing data corruption. Additionally, the implementation will include checks to prevent out-of-bounds memory access, which could otherwise lead to unexpected behavior or security vulnerabilities.
Copy link
Member

@jennijuju jennijuju Aug 22, 2024

Choose a reason for hiding this comment

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

Special care will be taken to ensure that overlapping memory regions are handled safely, preventing data corruption.

could you please elaborate on "the special care" that is taken to ensure the proper handling of memory region overlap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - the the spec is clear that if we copy one region to another and they overlap it should work as if you take a copy of the source region and paste it over the destination region without any errors or bugs due to over-writes as if you had created a unique copy of the memory being copied before the write. The original EIP describes it as: Copying takes place as if an intermediate buffer was used, allowing the destination and source to overlap. in the built in actors implementation we use the slice built in method copy_within which accounts for that issue.

@jennijuju
Copy link
Member

jennijuju commented Aug 22, 2024

@snissn thanks for the FIP! This draft proposes a straightforward fevm enhancement and is well-written! I have some small nits otherwise it LGTM

@jennijuju jennijuju changed the title Add Support for EIP-5656 (MCOPY Opcode) in the FEVM FIP-0094: Add Support for EIP-5656 (MCOPY Opcode) in the FEVM Aug 22, 2024
@snissn
Copy link
Contributor Author

snissn commented Aug 26, 2024

Thanks @jennijuju I was able to update the PR with your suggestions. Please review the response that I hope resolves your question about overlapping memory regions.

@@ -129,3 +129,4 @@ This improvement protocol helps achieve that objective for all members of the Fi
| [0090](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0090.md) | Non-Interactive PoRep | FIP | luca (@lucaniz), kuba (@Kubuxu), nicola (@nicola), nemo (@cryptonemo), volker (@vmx), irene (@irenegia) | Superseded by [FIP0092](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0092.md) |
| [0091](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0091.md) | Add support for Homestead and EIP-155 Ethereum Transactions ("legacy" Ethereum Transactions) | FIP | Aarsh (@aarshkshah1992)| Accepted |
| [0092](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0092.md) | Non-Interactive PoRep | FIP | luca (@lucaniz), kuba (@Kubuxu), nicola (@nicola), nemo (@cryptonemo), volker (@vmx), irene (@irenegia), Alex North (@anorth), orjan (@Phi-rjan) | Final |
| [XXXX](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-mcopy-opcode-fevm.md) | Add Support for EIP-5656 (MCOPY Opcode) in the FEVM | FIP | Michael Seiler (@snissn), Raúl Kripalani (@raulk), Steven Allen (@stebalien) | Draft |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| [XXXX](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-mcopy-opcode-fevm.md) | Add Support for EIP-5656 (MCOPY Opcode) in the FEVM | FIP | Michael Seiler (@snissn), Raúl Kripalani (@raulk), Steven Allen (@stebalien) | Draft |
| [0094](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-mcopy-opcode-fevm.md) | Add Support for EIP-5656 (MCOPY Opcode) in the FEVM | FIP | Michael Seiler (@snissn), Raúl Kripalani (@raulk), Steven Allen (@stebalien) | Draft |

@anorth anorth merged commit f295b4c into filecoin-project:master Sep 2, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

5 participants