codegen: use LibCodeGen.addressConstantString instead of a local copy - #11
Conversation
script/BuildPointers.sol hand-rolled an address-constant emitter that rain-sol-codegen already publishes as LibCodeGen.addressConstantString, in the same file that already calls LibCodeGen.bytesConstantString. The private copy hardcodes the comment text and the DEPLOYED_ADDRESS name; the shared one is parameterised on both. The pinned rain-sol-codegen 0.1.0 predates addressConstantString (it first shipped in 0.1.2), so the dependency moves to 0.1.3 — the newest version that leaves generated output byte-identical. 0.1.0 -> 0.1.3 is purely additive in src/lib (addressConstantString, bytes32ConstantString, LibSnapshot); 0.1.4 is not, because it renames LibFs.pathForContract's output from <name>.pointers.sol to <name>.sol and rewrites the generated header comment, which is a separate migration. Generated output is unchanged: the local emitter and the shared one produce the same bytes for this input, so no generated file moves and no frozen per-tag snapshot is rewritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThe PR updates ChangesCode generation integration
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change reuses the shared address-constant generator without changing generated output or production Solidity behavior; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
🤖 ai:vetter |
Closes #10
Motivation
script/BuildPointers.soldefined its ownaddressConstantStringforaddress constant DEPLOYED_ADDRESS = address(...)while, in the same file,calling
LibCodeGen.bytesConstantStringfromrain-sol-codegenfor the othergenerated constants.
LibCodeGenalready publishes the address emitter, and itsversion is strictly more general — parameterised on the comment text and the
constant name instead of hardcoding both.
Changes
script/BuildPointers.sol: localaddressConstantStringdeleted; the callsite now uses
LibCodeGen.addressConstantString(vm, <the same NatSpec>, "DEPLOYED_ADDRESS", deployed).rain-sol-codegen0.1.0->0.1.3(foundry.toml,soldeer.lock, the twoversioned import prefixes). Required: the pinned
0.1.0predatesaddressConstantString, which first shipped in published0.1.2.Why 0.1.3 and not latest
0.1.0 -> 0.1.3is purely additive insrc/lib— it addsaddressConstantString,bytes32ConstantStringandLibSnapshot.solandchanges nothing this repo already calls, so generated output cannot move.
0.1.4is not a candidate here. It changesLibFs.pathForContractfromsrc/generated/<name>.pointers.soltosrc/generated/<name>.soland rewritesthe
THIS FILE IS AUTOGENERATED BY ./script/BuildPointers.solheader emittedinto every generated file. Taking it would rename and rewrite the committed
per-tag snapshots — including frozen ones — which is a separate migration, not a
side effect of deduplicating one function.
Drift check: byte-identical, nothing regenerated
The issue asks whether this repo's private copy had drifted from the shared one.
It had not. Both emit
and the shared function's only extra behaviour is a line-wrap branch that fires
above 120 columns. This line is 88 columns
(
17 + 16 ("DEPLOYED_ADDRESS") + 3 (" = ") + 8 ("address(") + 42 + 2 (");")), sothe single-space branch is taken and the bytes match exactly.
Confirmed empirically, not just by reading:
forge script ./script/BuildPointers.sol && forge fmton this branch leavessrc/generated/completely clean. Nogenerated file moves, so no frozen
src/generated/<tag>/snapshot is rewritten—
0_1_3/,0_1_4/and0_1_5/are all untouched, and CI'sfrozen-snapshots-append-onlycheck independently enforces that.QA
forge script ./script/BuildPointers.sol && forge fmt && git diff --exit-code. It isdiscriminating for exactly this change because the committed
src/generated/0_1_5/CloneFactory.pointers.solwas produced by the OLDhand-rolled emitter, so any byte the new one emits differently shows up as a
diff.
script_rc=0 generated_files_dirty=0— proof the gateactually executed and was green, not merely assumed.
regenerated and re-checked against the committed artifacts:
deploy address->deployed address): KILLED,generated_files_dirty=1.DEPLOYED_ADDRESS->DEPLOYED_ADDR): KILLED,generated_files_dirty=1.\ncollapsed to a space): KILLED,generated_files_dirty=1.come back clean, so a silently-not-running gate cannot report kills.
output of the emitter being replaced. Byte-equality against them is the
whole correctness claim; it is checked, not reasoned about.
text, constant name and line wrapping are the complete set of inputs the
shared function's output depends on, since it is
pureover(comment, name, address)with one length-derived branch. The addressitself is covered by the unchanged
BYTECODE_HASH/CREATION_CODE/RUNTIME_CODEconstants generated beside it. No production Solidity changesin this PR: the only non-script edits are the dependency pin and its lock.
🤖 Generated with Claude Code
Summary by CodeRabbit