Skip to content

fix(core): handle invalid bytecode in getUnlinkedBytecode for library-linked contracts#1246

Open
BhariGowda wants to merge 2 commits into
OpenZeppelin:masterfrom
BhariGowda:fix/getUnlinkedBytecode-library-linked-contracts
Open

fix(core): handle invalid bytecode in getUnlinkedBytecode for library-linked contracts#1246
BhariGowda wants to merge 2 commits into
OpenZeppelin:masterfrom
BhariGowda:fix/getUnlinkedBytecode-library-linked-contracts

Conversation

@BhariGowda
Copy link
Copy Markdown

@BhariGowda BhariGowda commented May 16, 2026

Motivation

Fixes #1227

getUnlinkedBytecode() throws "Bytecode is not a valid hex string" when
a Hardhat build-info file contains contracts with external library link
references (e.g., LockupLib, ERC3643BatchLib), even when the contract
being deployed has no library dependencies.

Root Cause

The function iterates over all contracts with linkReferences and applies
their link references to the target bytecode via unlinkBytecode(). When
wrong link references are applied, the resulting bytecode is corrupted.
getVersion() then throws on the invalid hex string before the version
check can determine it's not a match.

Fix

Wrap the getVersion() call in a try/catch. When unlinkBytecode() with
wrong link references produces invalid bytecode, skip that candidate and
continue to the next one.

Testing

Added regression test. All 588 tests pass.

Summary by CodeRabbit

  • Bug Fixes

    • Improved robustness of bytecode validation to handle edge cases gracefully without interrupting the validation process.
  • Tests

    • Added regression test to verify bytecode validation handles complex contract dependency scenarios correctly.

Review Change Stack

…info contains library-linked contracts

When a build-info file contains contracts with external library link
references, getUnlinkedBytecode() would throw 'Bytecode is not a valid
hex string' for unrelated contracts with no library dependencies.

Root cause: unlinkBytecode() applies wrong link references to the target
bytecode, corrupting it. getVersion() then throws on the invalid hex.

Fix: wrap getVersion() call in try/catch and skip candidates that produce
invalid bytecode, matching the intended fallthrough behavior.

Fixes OpenZeppelin#1227
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d10ae20-6915-4401-927f-444dcb31134c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

This PR fixes a crash in getUnlinkedBytecode by wrapping version extraction in try/catch to skip invalid bytecode produced from incompatible link references, and adds a regression test to ensure the function returns target bytecode unchanged when unrelated contracts have in-bounds link references.

Changes

Bug fix for getUnlinkedBytecode crash

Layer / File(s) Summary
Error handling in version extraction
packages/core/src/validate/query.ts
getUnlinkedBytecode now catches exceptions from getVersion when invalid bytecode is produced from incompatible link references, allowing iteration to continue rather than failing at the first computed version.
Regression test for bytecode matching
packages/core/src/validate/query.test.ts
New test validates getUnlinkedBytecode does not throw when build-info contains library-linked contracts whose link-reference offsets fall within the target bytecode bounds, but the target contract itself has no library dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A bytecode crash did haunt the night,
When unrelated libs were linked just right;
With try/catch guards the code stands tall,
No more shall version-checks enthrall!
hops away

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main fix: wrapping getVersion() in try/catch to handle invalid bytecode when library-linked contracts are processed by getUnlinkedBytecode.
Linked Issues check ✅ Passed The PR fully addresses #1227 by guarding the getVersion() call with try/catch to skip invalid unlinked bytecode, continuing with other candidates, and preserving the fallback behavior of returning original bytecode.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing getUnlinkedBytecode(): code fix in query.ts and corresponding regression test in query.test.ts; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/core/src/validate/query.test.ts (1)

50-85: ⚡ Quick win

Add one more regression for the unlinkBytecode-throws path.

This test is solid for invalid-hex-after-unlinking. Adding a second case with out-of-bounds/malformed link references would lock in the non-fatal behavior for direct unlinkBytecode failures too.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/validate/query.test.ts` around lines 50 - 85, Add a second
regression test that directly exercises unlinkBytecode to ensure out-of-bounds
or malformed link references do not cause a fatal error: create a mock
validation entry (e.g., LibraryLinkedContract) with a linkReferences array whose
start/length exceed the provided bytecode bounds or contain malformed
placeholders, then call unlinkBytecode(validation as ValidationRunData,
targetBytecode) and assert it either throws a controlled, documented error
(using t.throws with the expected message) or safely returns the original
bytecode depending on the expected behavior; reference the existing test helpers
and symbols getUnlinkedBytecode, unlinkBytecode, ValidationRunData, and the
sample targetBytecode/SimpleContract setup so the new test mirrors the existing
structure but targets the unlinkBytecode failure path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/validate/query.ts`:
- Around line 132-142: The try/catch only wraps getVersion so unlinkBytecode
errors can still crash getUnlinkedBytecode; move the call to
unlinkBytecode(bytecode, linkReferences) inside the same try block that calls
getVersion (or wrap both in a single try) and on any exception continue to the
next candidate, ensuring the logic that checks
validation[name].version?.withMetadata remains the same.

---

Nitpick comments:
In `@packages/core/src/validate/query.test.ts`:
- Around line 50-85: Add a second regression test that directly exercises
unlinkBytecode to ensure out-of-bounds or malformed link references do not cause
a fatal error: create a mock validation entry (e.g., LibraryLinkedContract) with
a linkReferences array whose start/length exceed the provided bytecode bounds or
contain malformed placeholders, then call unlinkBytecode(validation as
ValidationRunData, targetBytecode) and assert it either throws a controlled,
documented error (using t.throws with the expected message) or safely returns
the original bytecode depending on the expected behavior; reference the existing
test helpers and symbols getUnlinkedBytecode, unlinkBytecode, ValidationRunData,
and the sample targetBytecode/SimpleContract setup so the new test mirrors the
existing structure but targets the unlinkBytecode failure path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3645d6fc-6068-44ac-9a34-95119c1bca39

📥 Commits

Reviewing files that changed from the base of the PR and between 7f3e4c6 and c8f8043.

📒 Files selected for processing (2)
  • packages/core/src/validate/query.test.ts
  • packages/core/src/validate/query.ts

Comment thread packages/core/src/validate/query.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getUnlinkedBytecode crashes with "Bytecode is not a valid hex string" when build-info contains library-linked contracts

1 participant