-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
cannon: Implement 64-bit Solidity VM #12665
Conversation
7010229
to
7a4d3d2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #12665 +/- ##
===========================================
- Coverage 63.99% 63.62% -0.38%
===========================================
Files 55 55
Lines 4605 4629 +24
===========================================
- Hits 2947 2945 -2
- Misses 1475 1502 +27
+ Partials 183 182 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
7a4d3d2
to
1d8d82e
Compare
- Implements 64-bit Cannon (with multithreading) in MIPS64.sol - Re-enable differential testing for 64-bit VMs
1d8d82e
to
4046108
Compare
packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/cannon/libraries/MIPS64Syscalls.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/cannon/libraries/MIPS64Syscalls.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/cannon/libraries/MIPS64Syscalls.sol
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another pass comparing on/off-chain impls. Left a few more nits which are pretty minor. I think we should probably add a test around the InvalidPC error. I would feel better if we had more differential tests on some of the math, but if you feel strongly we can always follow up with more testing in later PRs.
packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol
Outdated
Show resolved
Hide resolved
packages/contracts-bedrock/src/cannon/libraries/MIPS64Instructions.sol
Outdated
Show resolved
Hide resolved
* cannon: Implement MIPS64Memory.sol * cannon: Implement 64-bit Solidity VM - Implements 64-bit Cannon (with multithreading) in MIPS64.sol - Re-enable differential testing for 64-bit VMs * review comments * check pc for 4-byte alignment * gofmt * update snapshot * address nits; add more add/sub/mult overflow tests * diff test misaligned instruction * fix mul[t] MIPS64.sol emulation * diff fuzz mul operations * fix addiu test case * fix GetInstruction return value type
* cannon: Implement MIPS64Memory.sol * cannon: Implement 64-bit Solidity VM - Implements 64-bit Cannon (with multithreading) in MIPS64.sol - Re-enable differential testing for 64-bit VMs * review comments * check pc for 4-byte alignment * gofmt * update snapshot * address nits; add more add/sub/mult overflow tests * diff test misaligned instruction * fix mul[t] MIPS64.sol emulation * diff fuzz mul operations * fix addiu test case * fix GetInstruction return value type
Introduces a new VM smart contract that implements 64-bit Cannon (with multitasking). This is implemented in MIPS64.sol.
Testing
Differential testing of both 64-bit VMs is re-enabled. These tests can be run using
make -C ./cannon test64
.These tests include:
These tests don't yet run in CI because there are existing diff tests for the cannon32 build that aren't 64-bit compatible. Fixing those tests and CI integration will be addressed in another PR.
Meta
Fixes #12250