Pin the four tooling interface ids and inherit them from a mock

[thedavidmeister]

Closes #74.

The four published tooling interfaces are implemented by contracts in other
repositories and probed over ERC-165 by contracts already deployed on chain, and
nothing in this repository held their ids. src/interface/ had no test file at
all, so a rename or a signature change carried through every implementer moved
type(I…).interfaceId off what is deployed while every downstream ERC-165 test
recomputed the constant from the same source and stayed green.

test/src/interface/I*.t.sol — one file per interface, mirroring
src/interface/ under the placement rule — pins each id to a literal and holds
ToolingMock against the interface it implements.

interface	pinned id
IOpcodeToolingV1	0x514b5d4f
IParserToolingV1	0x1a2c8edd
ISubParserToolingV1	0x336284d4
IIntegrityToolingV1	0xb92d7553

The four literals were not taken from the issue. nix develop -c cast sig gives
buildOpcodeFunctionPointers() 0x514b5d4f, buildSubParserWordParsers()
0x336284d4, buildIntegrityFunctionPointers() 0xb92d7553,
buildLiteralParserFunctionPointers() 0xccf44775 and
buildOperandHandlerFunctionPointers() 0xd6d8c9a8. The first three interfaces
declare one function each, so their id is that selector; IParserToolingV1
declares two, and 0xccf44775 ^ 0xd6d8c9a8 = 0x1a2c8edd. All four agree with the
issue's numbers, and none of them moved when #124 changed the builders to view
— state mutability is not part of a selector.

ConformingToolingMock is deleted and the tests hold ToolingMock instead

This branch previously added a second mock. Its only reason to exist was that
IParserToolingV1 and ISubParserToolingV1 declared their three builders
pure, so no per-instance mock could implement them and ToolingMock could not
inherit the interfaces at all. #124 has since landed: all five builders are
view, and ToolingMock declares is IOpcodeToolingV1, IParserToolingV1, ISubParserToolingV1, IIntegrityToolingV1 with override on all five. The reason
is gone, so the mock goes with it.

The case for keeping it was that a pure implementation conforming to a view
declaration is a distinct property from a storage-reading one. It is distinct,
but it is a property of the compiler rather than of this repository: an override
may only tighten state mutability, and pure is the tightest, so a pure
implementation conforms to every declaration that is not payable. That makes it
strictly the weaker instrument, and this was measured rather than reasoned.
Restoring ConformingToolingMock next to ToolingMock and tightening each
builder's declaration to pure in turn, forge build cites:

view -> pure on IOpcodeToolingV1.buildOpcodeFunctionPointers
  Error (6959) --> test/concrete/ConformingToolingMock.sol:45
  Error (6959) --> test/concrete/ToolingMock.sol:43
view -> pure on IParserToolingV1.buildOperandHandlerFunctionPointers
  Error (6959) --> test/concrete/ToolingMock.sol:53
view -> pure on IParserToolingV1.buildLiteralParserFunctionPointers
  Error (6959) --> test/concrete/ToolingMock.sol:48
view -> pure on ISubParserToolingV1.buildSubParserWordParsers
  Error (6959) --> test/concrete/ToolingMock.sol:58
view -> pure on IIntegrityToolingV1.buildIntegrityFunctionPointers
  Error (6959) --> test/concrete/ConformingToolingMock.sol:50
  Error (6959) --> test/concrete/ToolingMock.sol:63

ToolingMock is named on all five. ConformingToolingMock is named on two, and
is blind to the three it implements pure — the three the issue was worried
about. There is no mutation of the four interfaces it catches that ToolingMock
does not, and three that ToolingMock catches and it does not.

Ledger for the deletion. Build cost is nil — a git rm and four import lines.
Carrying cost is a second 68-line mock kept in step with five builder signatures,
answering three of them from constants no test pins, and a standing question at
every future interface change about which of two mocks is the one that matters.
Cost of bringing it back is the same git rm in reverse, and the condition that
would call for it — an interface going back to declaring a builder pure — makes
ToolingMock stop compiling and say so, which is the same signal by a shorter
route.

What replaces it is not an address cast. Each test reaches the interface type
by assignment — IOpcodeToolingV1 tooling = mock; — which compiles only while
ToolingMock actually inherits that interface. The I…(address(mock)) form the
earlier revision used compiles whether or not anything inherits anything, which
is the hole the issue names: "tests cast a bare address to the interface at the
call site". Dropping an interface from ToolingMock's inheritance list along
with its override and @inheritdoc — the shape a "simplify the mock" change
takes — leaves every test/lib call site green, and is caught only by these four
lines.

Divergence from the issue's proposed fix

The issue sketches one testToolingInterfaceIds() holding all four ids, plus a
new ConformingToolingMock. This ships four files rather than one, because the
placement rule keys test/src/** on the subject's path; and it ships no new mock
at all, because the mock the issue asks for now exists as ToolingMock.

QA

• Merged origin/main (b422d97, carrying Declare every tooling builder view so an implementation can read state #124) into this branch as f6910ca.
  The merge is textually clean, and the full suite on the merge commit — before
  any of this revision's changes — is Ran 22 test suites in 2.99s (25.60s CPU time): 150 tests passed, 0 failed, 0 skipped (150 total tests), so the merge
  is checked by a run rather than by the absence of conflict markers.

• Full suite after the revision, on the flake toolchain: Ran 22 test suites in 1.87s (19.21s CPU time): 150 tests passed, 0 failed, 0 skipped (150 total tests). The four interface suites are 8 tests passed, 0 failed, each fuzz
  test at runs: 2048. nix develop -c forge fmt --check exits 0.

• Mutation matrix: 45 mutants, 45 killed, 0 survived. Every mutant was applied
  to a committed-clean tree, scored by a full forge test with
  cache/fuzz/failures deleted first so no counterexample replay could stand in
  for a discovery, and reverted with git checkout -- after. The unmutated tree
  was scored through the same harness first and reported rc=0 with
  total_ran=150 — which is what rules out a harness error reading as a clean
  pass. git status is empty after the run.

  A. Interface mutants killed at compile time (30). For each of the five
  builders across the four interfaces: rename, return type bytes memory →
  bytes32, add a uint256 argument, view → pure, view → nonpayable. All
  rc=1, total_ran=0. The two mutability directions are caught by two
  different mechanisms, both read out of the compiler:

  view -> pure         Error (6959): Overriding function changes state mutability from "pure" to "view".
                         --> test/concrete/ToolingMock.sol:43   buildOpcodeFunctionPointers
                         --> test/concrete/ToolingMock.sol:48   buildLiteralParserFunctionPointers
  view -> nonpayable   Error (8961): Function cannot be declared as view because this expression (potentially) modifies the state.
                         --> src/lib/LibCodeGen.sol:132         interpreter.buildOpcodeFunctionPointers()
                         --> src/lib/LibCodeGen.sol:158         instance.buildLiteralParserFunctionPointers()
  

  A tightening is caught by the mock; a loosening is caught by LibCodeGen's own
  view wrappers. Both directions are red, and neither needed a second mock.

  B. Adding a builder to each interface (4). Error (3656): Contract "ToolingMock" should be marked as abstract. on all four.

  C. Flipping each pinned id literal by one nibble (4). All four compile and
  run — total_ran=150 — and fail on the pin alone.

  D. Dropping each interface from ToolingMock's inheritance, with its
  override and @inheritdoc (4). All four are Error (9574): Type contract ToolingMock is not implicitly convertible to expected type contract I…, cited
  at the new assignment line — IOpcodeToolingV1.t.sol:38,
  IParserToolingV1.t.sol:47, ISubParserToolingV1.t.sol:38,
  IIntegrityToolingV1.t.sol:38. Nothing else in the suite notices these.

  E. Coherent mutants, where the whole tree still compiles (8). These are what
  the pin exists for: a change carried through every implementer and every caller
  leaves the suite green otherwise. All eight run total_ran=150 and fail only
  on the id test.

  mutant	id observed	pinned
  buildOpcodeFunctionPointers renamed across src and test	0xb0076584	0x514b5d4f
  buildLiteralParserFunctionPointers renamed across src and test	0xb1dc4421	0x1a2c8edd
  buildSubParserWordParsers renamed across src and test	0x89ff7ed1	0x336284d4
  buildIntegrityFunctionPointers renamed across src and test	0x9cf38df8	0xb92d7553
  a builder added to IOpcodeToolingV1 and implemented on ToolingMock	0xebcf676f	0x514b5d4f
  a builder added to IParserToolingV1 and implemented on ToolingMock	0xc53e923c	0x1a2c8edd
  a builder added to ISubParserToolingV1 and implemented on ToolingMock	0x41e370a0	0x336284d4
  a builder added to IIntegrityToolingV1 and implemented on ToolingMock	0xb563b4c4	0xb92d7553

  The last four are the issue's own scenario verbatim — a sixth builder lands
  without minting a V2, everything in every repository recompiles, and the only
  thing that goes red is the literal.

• Oracle: the ABI encoding rules and ERC-165, not the source. Each pinned id was
  derived from cast sig over the five builder signatures and, for
  IParserToolingV1, an exclusive or done by hand — never read back from
  type(I…).interfaceId, which is the expression under test.

• Category check: the category is every interface published from
  src/interface/. ls src/interface returns exactly IIntegrityToolingV1.sol,
  IOpcodeToolingV1.sol, IParserToolingV1.sol, ISubParserToolingV1.sol, and
  find test/src/interface -name '*.t.sol' returns exactly four matching files,
  so the set is closed on the rule rather than on the issue's list.

• Not asserted, and deliberately: nothing pins which of the two selectors
  IParserToolingV1's id exclusive-ors together carries which meaning, because
  the id genuinely cannot say. What holds the two builders apart is that each is
  read back through its own name against its own value, both here and in
  test/lib/LibCodeGen.*ConstantString.t.sol.

[coderabbitai]

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 57 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 97a6e650-e875-4e64-9b7b-36d4af7567e7

📥 Commits

Reviewing files that changed from the base of the PR and between b422d97 and e4ece78.

📒 Files selected for processing (4)

• test/src/interface/IIntegrityToolingV1.t.sol
• test/src/interface/IOpcodeToolingV1.t.sol
• test/src/interface/IParserToolingV1.t.sol
• test/src/interface/ISubParserToolingV1.t.sol

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thedavidmeister]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.