Feat/debug witness memory issue

[georgehao]

1. Purpose or design rationale of this PR

Repeated calls to debug_executionWitness lead to unbounded memory growth, eventually leading to OOM. Tests show. that this is fixed by removing testWitness. We're still investigating why this is the case.

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

• fix: A bug fix

3. Deployment tag versioning

Has the version in params/version.go been updated?

• This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
• Yes

4. Breaking change label

Does this PR have the breaking-change label?

• This PR is not a breaking change
• Yes

Summary by CodeRabbit

• Refactor

  • Adjusted witness generation to skip an internal post-construction validation step. Behavior and public interfaces remain unchanged; no user action required.

• Chores

  • Patch version bumped to 5.9.5. Updated version string appears in CLI output, metadata, and archive identifiers.

[coderabbitai]

Walkthrough

Removes the post-construction invocation of testWitness from generateWitness in eth/api.go, so generateWitness returns the constructed witness without running testWitness. Also increments VersionPatch from 4 to 5 in params/version.go, updating the computed version to 5.9.5.

Changes

Cohort / File(s)	Summary
Witness generation validation flow eth/api.go	Removed the call to testWitness inside generateWitness; testWitness remains defined but is no longer invoked by generateWitness.
Version bump params/version.go	Updated VersionPatch from 4 to 5, changing Version to 5.9.5 (affects VersionWithMeta / ArchiveVersion).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant API as generateWitness
  participant TW as testWitness

  rect rgb(235,245,255)
  note over API: Previous flow
  C->>API: generateWitness()
  API->>API: construct witness
  API->>TW: testWitness(witness)
  TW-->>API: ok / error
  alt validation failed
    API-->>C: error
  else validation ok
    API-->>C: witness
  end
  end

  rect rgb(240,255,240)
  note over API: New flow
  C->>API: generateWitness()
  API->>API: construct witness
  note over API: testWitness call removed
  API-->>C: witness
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: ignore key-not-found errors in testWitness #1164 — Modifies testWitness error handling; relates to removal of the generateWitness→testWitness interaction.
• fix: an attempt at making witness generation more stable #1166 — Alters witness generation/validation flow in eth/api.go; touches same area.
• fix: avoid using snapshots when generating witness #1116 — Changes state DB acquisition and error handling used during witness generation.

Suggested labels

bump-version

Suggested reviewers

• omerfirmak
• colinlyguo
• jonastheis

Poem

I nibble code and hop with glee,
The witness skips its last decree.
A patch bumped up—five-oh-five,
I thump my foot, the build’s alive. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	Title 'Feat/debug witness memory issue' directly references the primary change—witness handling and debugging a memory issue—and aligns with the changeset that alters generateWitness behavior. It is concise and focused on the main intent. It does not strictly follow the repository's Conventional Commits casing/colon format, so a minor reformat is recommended.
Description Check	✅ Passed	The PR description includes the required template sections: a Purpose that explains repeated calls to debug_executionWitness cause unbounded memory growth and that removing testWitness appears to fix it, the PR-type checklist marked as "fix", deployment/versioning marked "Yes", and the breaking-change box marked as not breaking; the code changes also include the params/version.go bump. Overall the description is largely complete and actionable for reviewers. However the repository PR title shown in the PR metadata ("Feat/debug witness memory issue") does not follow conventional-commit formatting and conflicts with the selected 'fix' type, and there is a small grammar typo in the purpose paragraph.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/debug_witness_memory_issue

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between afbbad7 and 65024a9.

📒 Files selected for processing (2)

• eth/api.go (1 hunks)
• params/version.go (1 hunks)

🧰 Additional context used

🪛 GitHub Actions: CI

eth/api.go

[error] 385-385: golangci-lint: testWitness is unused (deadcode). Step: lint (env GO111MODULE=on go run build/ci.go lint).

🔇 Additional comments (1)

params/version.go (1)

27-27: Approve patch bump to 5.9.5

Patch bump validated—no occurrences of “5.9.4” remain. Ensure deployment checklist is ticked and release notes/changelog reference 5.9.5.

[Thegaram]

LGTM