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

  • Witness generation now skips an internal post-construction validation step; external behavior and public interfaces are unchanged.

• Chores

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

[coderabbitai]

Walkthrough

Removes the post-construction invocation of testWitness from generateWitness in eth/api.go, so generateWitness now returns the constructed witness without calling testWitness. Also increments VersionPatch from 10 to 11 in params/version.go, updating the computed version to 5.9.11.

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 10 to 11, changing Version to 5.9.11 (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

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review focus: eth/api.go witness construction logic and any callers that may have relied on post-construction validation; params/version.go version constant change and where Version is consumed.

Possibly related PRs

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

Suggested labels

bump-version

Suggested reviewers

• Thegaram
• omerfirmak
• colinlyguo
• jonastheis

Poem

I nibble bytes beneath moonlight,
The witness hops away tonight.
A patch to eleven, neat and spry,
I twitch my whiskers, give a sigh—
Build hops on, carrot held up high. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'Feat/debug witness memory issue' does not follow conventional commits format; it should start with 'fix:' per the author's own selection in the description template.	Rename the PR title to 'fix: debug witness memory issue' to comply with conventional commits and match the 'fix' type selected by the author.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description includes all required sections from the template: purpose/rationale, conventional commits compliance confirmation, versioning update confirmation, and breaking change status.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f864b60 and 5cc3803.

📒 Files selected for processing (1)

• params/version.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• params/version.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: test
• GitHub Check: Analyze (go)

---

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

[Thegaram]

@georgehao Do you plan to merge this?