cannon: 64-bit Refactor

[Inphi]

Overview

This is a refactor of Cannon to support both 32-bit and 64-bit MIPS emulation while reusing code as much as possible.
A lot of the implementation was guided by the initial efforts in #11388 and @GrapeBaBa for anton-rs/cannon-rs#36.
The refactor prioritizes readability and avoids duplicating code to ensure that the 32-bit behavior is not altered. This is achieved by partitioning 64-bit and 32-bit logic using go build tags. The cannon32 and cannon64 build tags are used to implement architecture specific functionality. Most MIPS emulation logic can be factorized into common constructs with minimal loss in readability. For example, exec.SignExtend can be implemented across all VMs with the following:

func SignExtend(dat arch.Word, idx arch.Word) arch.Word {
        isSigned := (dat >> (idx - 1)) != 0
        signed := ((arch.Word(1) << (arch.WordSize - idx)) - 1) << idx
        mask := (arch.Word(1) << idx) - 1
        if isSigned {
                return dat&mask | signed
        } else {
                return dat & mask
        }
}

Where arch is a new cannon package that abstracts away the specifics of either MIPS32 or MIPS64.

One advantage of this approach is that MT-Cannon can be gradually ported over to MIPS64 without breaking the altering the existing code structure. Similarly, the single-threaded Cannon VM can be ported over to MIPS64 (though this isn't desired). This unlocks parallel development of 32 and 64 bit VMs.

This refactor does not affect the MIPS solidity contracts. The goal is to retain readability such that the refactored Go implementations can still be easily compared with the solidity counterparts. Introducing a 64-bit MIPS VM in Solidity will be done later.

Concerns

The downside of this approach is that both 32-bit and 64-bit VMs can no longer exist in the same Cannon binary. This breaks other packages, including the op-challenger, that rely on cannon packages for state decoding. Deployment of the op-challenger is further complicated as two VM binaries are needed for both single-threaded MIPS32 and MT-MIPS64 emulation. Fortunately this issue can be solved using multicannon which lets us select either 32-bit or 64-bit VM implementations depending on the CLI inputs.

Testing Plan

Note that the MIPS64 implementation is not complete. The primary goal of this PR is to ensure that the MIPS32 VM behaviors are not altered. Comprehensive MIPS64 testing will be added later.

• Modify existing tests to assert MIPS32 functionality.
• Differentially test the refactored cannon against the version on develop using a complex program (ex: op-program).
  • Done for simpler programs but not the op-program

[semgrep-app]

Semgrep found 13 sol-style-notice-over-dev-natspec findings:

• packages/contracts-bedrock/src/dispute/interfaces/IInitializable.sol
  • L8 - Triage
• packages/contracts-bedrock/src/dispute/interfaces/IFaultDisputeGame.sol
  • L51 - Triage
  • L73 - Triage
• packages/contracts-bedrock/src/dispute/interfaces/IDisputeGameFactory.sol
  • L42 - Triage
  • L97 - Triage
  • L103 - Triage
  • L109 - Triage
• packages/contracts-bedrock/src/dispute/interfaces/IDisputeGame.sol
  • L28 - Triage
  • L34 - Triage
  • L39 - Triage
  • L44 - Triage
  • L49 - Triage
  • L57 - Triage

Prefer @notice over @dev in natspec comments

_{Ignore this finding from sol-style-notice-over-dev-natspec.}

[ajsutton]

Very cool idea. I think it's worth having a wrapper of some form so that op-challenger can just invoke cannon and have it just work. I'm trying to reduce the amount challenger has to understand about the VM states so would like to avoid the auto detection happening in challenger and otherwise we're back to needing a new game type which adds a bunch of unfortunate overhead. Hopefully that's not too hard to do because this looks like a pretty big win in being able to make incremental progress towards 64 bit and avoiding a lot of code duplication.

[codecov]

Codecov Report

Attention: Patch coverage is 65.04348% with 201 lines in your changes missing coverage. Please review.

Project coverage is 71.99%. Comparing base (d05fb50) to head (378e22e).
Report is 3 commits behind head on develop.

Files with missing lines	Patch %	Lines
cannon/mipsevm/exec/mips_instructions.go	46.12%	122 Missing and 3 partials ⚠️
cannon/mipsevm/versions/state.go	21.62%	24 Missing and 5 partials ⚠️
cannon/mipsevm/memory/memory.go	86.66%	3 Missing and 5 partials ⚠️
cannon/mipsevm/multithreaded/mips.go	80.00%	5 Missing and 2 partials ⚠️
cannon/mipsevm/exec/mips_syscalls.go	86.20%	4 Missing ⚠️
cannon/mipsevm/multithreaded/state.go	76.47%	2 Missing and 2 partials ⚠️
cannon/mipsevm/multithreaded/testutil/mutators.go	70.00%	3 Missing ⚠️
cannon/mipsevm/singlethreaded/instrumented.go	25.00%	2 Missing and 1 partial ⚠️
cannon/mipsevm/testutil/rand.go	72.72%	2 Missing and 1 partial ⚠️
cannon/mipsevm/testutil/state.go	70.00%	2 Missing and 1 partial ⚠️
... and 9 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #12029      +/-   ##
===========================================
- Coverage    75.19%   71.99%   -3.21%     
===========================================
  Files           49       50       +1     
  Lines         3665     3910     +245     
===========================================
+ Hits          2756     2815      +59     
- Misses         736      910     +174     
- Partials       173      185      +12     

Flag	Coverage Δ
cannon-go-tests	71.99% <65.04%> (-3.21%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cannon/mipsevm/arch/arch32.go	100.00% <100.00%> (ø)
cannon/mipsevm/exec/preimage.go	87.87% <100.00%> (ø)
cannon/mipsevm/exec/stack.go	66.66% <100.00%> (ø)
cannon/mipsevm/memory/page.go	66.21% <100.00%> (ø)
cannon/mipsevm/multithreaded/testutil/thread.go	99.15% <100.00%> (ø)
cannon/mipsevm/multithreaded/thread.go	61.17% <100.00%> (ø)
cannon/mipsevm/program/patch.go	86.56% <100.00%> (+2.63%)	⬆️
cannon/mipsevm/singlethreaded/mips.go	93.24% <100.00%> (ø)
cannon/mipsevm/singlethreaded/state.go	55.66% <100.00%> (ø)
cannon/mipsevm/state.go	100.00% <ø> (ø)
... and 23 more

... and 1 file with indirect coverage changes

[semgrep-app]

Semgrep found 2 sol-style-require-reason findings:

• packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
  • L1244 - Triage
  • L1328 - Triage

require() must include a reason string

_{Ignore this finding from sol-style-require-reason.}

Semgrep found 12 sol-safety-deployutils-args findings:

• packages/contracts-bedrock/scripts/deploy/Deploy.s.sol
  • L677-682 - Triage
  • L693-698 - Triage
  • L717-722 - Triage
  • L777-782 - Triage
  • L803-808 - Triage
  • L824-829 - Triage
  • L871-876 - Triage
  • L950-955 - Triage
  • L958-963 - Triage
  • L977-982 - Triage
  • 2 more - Triage

_args parameter should be wrapped with DeployUtils.encodeConstructor

_{Ignore this finding from sol-safety-deployutils-args.}

[clabby]

This looks really good. Awesome job!

Approving as this looks good per the original 64-bit changes I made in #11388. Probably good to get eyes from @mbaxter + @ajsutton as well before merging + moving forward with the 64-bit smart contract from #11404.

P.S. - there's also some 64-bit ASM tests from #11388 we can use.

[ajsutton]

LGTM generally. I assume at some point we'll have 64bit MIPS reference tests we can pull in to verify each instruction is implemented correctly. That's quite hard to review manually.

Otherwise mostly just ensuring we have issues referenced for the TODOs (I'd just point them all to the 64-bit support tracker issue and not try to log individual issues for everything necessarily). And we'll have to sort out some conflicts with the getfd support but I don't think that will be a problem.

[mbaxter]

Overall looks really good!

Would definitely be nice to get the additional differential tests in for extra confidence that none of the 32-bit behavior has changed. I did end up kind skimming over some of the new 64-bit ops - might try to spend some more time looking through those later.

[semgrep-app]

Semgrep found 8 golang_fmt_errorf_no_params findings:

• op-e2e/external_geth/main.go
  • L35 - Triage
  • L50 - Triage
  • L53 - Triage
  • L70 - Triage
  • L106 - Triage
  • L137 - Triage
  • L171 - Triage
  • L175 - Triage

No fmt.Errorf invocations without fmt arguments allowed

_{Ignore this finding from golang_fmt_errorf_no_params.}

Semgrep found 3 dangerous-exec-command findings:

• op-e2e/system/e2esys/external.go
  • L111 - Triage
• op-e2e/external_geth/main.go
  • L112-117 - Triage
  • L139-161 - Triage

Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.

_{Ignore this finding from dangerous-exec-command.}

[Inphi]

@mbaxter I've kicked off the cannon-stf-verify job to assert that this change doesn't break the VM STF. It uses this PR and cannon/v1.1.0-alpha.1 to compute the post-exit state of the hello.go program and compares the results. It passes which provides some confidence that we haven't broken anything.