Skip to content

refactor(extern): add INTEGRITY_FUNCTION_POINTERS_LENGTH and pin pointer slot non-zero tests - #226

Open
thedavidmeister wants to merge 2 commits into
mainfrom
2026-06-24-issue-28-32-pointer-table
Open

refactor(extern): add INTEGRITY_FUNCTION_POINTERS_LENGTH and pin pointer slot non-zero tests#226
thedavidmeister wants to merge 2 commits into
mainfrom
2026-06-24-issue-28-32-pointer-table

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

Closes #28
Closes #32

What changed

Issue #28 — mis-named constant for integrity table size
buildIntegrityFunctionPointers() was allocating its array using OPCODE_FUNCTION_POINTERS_LENGTH. This hides any future divergence between opcode-table size and integrity-table size (BaseRainlangExtern explicitly checks they match). Added a distinct INTEGRITY_FUNCTION_POINTERS_LENGTH = 2 constant and used it in buildIntegrityFunctionPointers(). OPCODE_FUNCTION_POINTERS_LENGTH is unchanged for backward compatibility.

Issue #32 — opcode/integrity index constants not pinned for non-zero values
Added two tests to ERC4626Words.indexAlignment.t.sol:

  • testOpcodeTableSlotsNonZero: each byte-pair at OPCODE_ERC4626_CONVERT_TO_ASSETS and OPCODE_ERC4626_CONVERT_TO_SHARES offsets in OPCODE_FUNCTION_POINTERS must be non-zero.
  • testIntegrityTableSlotsNonZero: same for INTEGRITY_FUNCTION_POINTERS.
    These guard against silently zero-defaulted function pointer slots that would compile fine but dispatch to address zero at runtime.

@thedavidmeister thedavidmeister self-assigned this Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 51 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

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

Run ID: edf0a147-1b1a-47ca-97bd-3083b4bd36fb

📥 Commits

Reviewing files that changed from the base of the PR and between 7d1eb1c and a01f94e.

📒 Files selected for processing (2)
  • src/abstract/ERC4626Extern.sol
  • test/src/concrete/ERC4626Words.indexAlignment.t.sol
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-06-24-issue-28-32-pointer-table

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

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

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Design-gate reject (human, 2026-07-04): the opcode and integrity tables are parallel-indexed over the SAME opcode set and can never legitimately differ in length — BaseRainlangExtern (rainlang-0.1.2 src/abstract/BaseRainlangExtern.sol:32-40) structurally enforces equality. A single shared constant is the correct encoding of that invariant (divergence unrepresentable); splitting it into INTEGRITY_FUNCTION_POINTERS_LENGTH CREATES a representable divergence plus the need for agreement checks — the two-sources-of-truth hazard introduced, not fixed. #28's premise is inverted; it is queued for human close. Only defensible kernel: an optional cosmetic rename of the shared constant to something like OPCODE_COUNT to express that one count drives both tables. Do not re-attempt the split.

@thedavidmeister thedavidmeister added the human:needs-work Human reviewer: needs rework label Jul 6, 2026
@thedavidmeister thedavidmeister added ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling) and removed human:needs-work Human reviewer: needs rework labels Jul 30, 2026
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

🤖 ai:vetter
vet-protocol 4
lens source@a01f94e40c79c0ea8c8367fd4430170f0b83db09 + audit skill invoked at pr:226
Reviewed a01f94e: needs-work — #28/#32: missing QA evidence — the PR body lacks the QA-GUIDE section-8 block; the code itself checks out (distinct INTEGRITY_FUNCTION_POINTERS_LENGTH sizes the integrity array per #28's option (b), and the new indexAlignment tests pin table lengths and non-zero pointer slots per #32 with correct 2-byte-slot semantics), so adding the evidence block is the remaining work.
cost 310 — missing QA evidence block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:needs-work Needs rework — the producer's inbox (vetter verdict or human ruling)

Projects

None yet

1 participant