Skip to content

fix(compilers): match the full 40-char library placeholder when stripping - #266

Open
ara-stock wants to merge 1 commit into
foundry-rs:mainfrom
ara-stock:fix-placeholder-regex
Open

ara-stock wants to merge 1 commit into
foundry-rs:mainfrom
ara-stock:fix-placeholder-regex

Conversation

@ara-stock

Copy link
Copy Markdown
Contributor

Problem

replace_placeholders_and_decode (used by BytecodeObject::strip_bytecode_placeholders) uses the pattern _\$.{34}\$_. A solc library placeholder is 40 characters, __$<34 hex chars>$__, so the pattern matches only the inner 38 characters and leaves one _ on each side. On top of that, the replacement is "00".repeat(40) (80 hex chars, 40 bytes) rather than the 20 bytes of an address. Because of the leftover _, hex decoding always fails, so strip_bytecode_placeholders returns None for every unlinked bytecode that contains a placeholder.

Fix

  • Match the exact placeholder: __\$[0-9a-fA-F]{34}\$__, the same __$<hash>$__ form that utils::library_hash_placeholder produces and link searches for.
  • Replace it with 20 zero bytes ("00".repeat(20), 40 hex chars), so the decoded bytecode keeps its length and the offsets of the code after it stay the same.

Related: foundry-rs/foundry#12656 tried to fix only the replacement length and was closed without merge in 2025-11. With that change alone the leftover _ characters still break decoding, so this PR fixes both the pattern and the replacement.

Test

Added strip_bytecode_placeholders_replaces_library_placeholder. It builds 6000 + a real placeholder for src/Lib.sol:Lib + 6000, and checks that replace_placeholders_and_decode returns 24 bytes with 20 zero bytes in place of the placeholder and that strip_bytecode_placeholders returns Some with the same bytes. Before this change it fails with InvalidHexCharacter { c: '_', index: 4 }.

cargo test -p foundry-compilers-artifacts-solc --lib: 63 passed. cargo fmt --all -- --check and cargo clippy -p foundry-compilers-artifacts-solc --all-targets are clean.

This PR was prepared with AI assistance (Claude Code); I reviewed the change and ran the tests.

🤖 Generated with Claude Code

…ping

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
}

// Replace all __$xxx$__ placeholders with 32 zero bytes
// Replace all __$xxx$__ placeholders with 20 zero bytes

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: the inline comment in strip_bytecode_placeholders (L353) still says "32 zero bytes"; could fix or drop it while here.

pub fn replace_placeholders_and_decode(s: &str) -> Result<Vec<u8>, hex::FromHexError> {
let re = regex::Regex::new(r"_\$.{34}\$_").expect("invalid regex");
let s = re.replace_all(s, "00".repeat(40));
let re = regex::Regex::new(r"__\$[0-9a-fA-F]{34}\$__").expect("invalid regex");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: the regex is recompiled on every call; a static LazyLock<Regex> would avoid that.

BytecodeObject::Unlinked("60__".into())
);
}
#[test]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: missing blank line before #[test].

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.

2 participants