codegen: use LibCodeGen.addressConstantString instead of a local copy - #308
codegen: use LibCodeGen.addressConstantString instead of a local copy#308thedavidmeister wants to merge 1 commit into
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 (3)
📝 WalkthroughWalkthroughThe project updates ChangesCodegen emitter update
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized dependency and code-generation cleanup preserves the existing generated output and snapshot behavior, so no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: Poem
🚥 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 |
Closes #307
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;buildContractPointersnow usesLibCodeGen.addressConstantString(vm, <the same NatSpec>, "DEPLOYED_ADDRESS", deployed).rain-sol-codegen0.1.0->0.1.3(foundry.toml,soldeer.lock,remappings.txt, the two versioned import prefixes). Required: the pinned0.1.0predatesaddressConstantString, which first shipped in published0.1.2. The now-danglingrain-sol-codegen-0.1.0/remapping is dropped;forge soldeer updateadds the new entry but does not remove the stale one.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, and matters more in this repo than in the othertwo. It changes
LibFs.pathForContractfrom<name>.pointers.solto<name>.soland rewrites theTHIS FILE IS AUTOGENERATED BY ./script/BuildPointers.solheader emitted into every generated file. Taking itwould rename and rewrite files inside the frozen
src/generated/0_1_1/snapshot, which downstream pins, and would break
LibProdDeployV4.sol's./<tag>/<name>.pointers.solimport paths. That is a separate migration, not aside 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 — the12
candidate/pointer files,LibProdDeployV4.solandLibProdDeployCurrent.solall regenerate to the bytes already committed.No frozen snapshot is rewritten.
deployTag()is the rollingcandidatetag, so the script only ever writes
src/generated/candidate/; the frozennumbered snapshot
src/generated/0_1_1/is read (to emit its aliases intoLibProdDeployV4.sol) and never written. It came through the regenerationuntouched, and CI's
frozen-snapshots-append-onlycheck enforces thatindependently.
QA
Git is cleanworkflow, run verbatim locally:forge script ./script/BuildPointers.sol && forge fmt && git diff --exit-code. It isdiscriminating for exactly this change because the committed
src/generated/candidate/*.pointers.solwere 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=12.DEPLOYED_ADDRESS->DEPLOYED_ADDR): KILLED,generated_files_dirty=12.\ncollapsed to a space): KILLED,generated_files_dirty=12.harness aborts if the unmutated baseline does not 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, and the downstreamLibProdDeployV4/LibProdDeployCurrentaliases are regenerated in the samerun. No production Solidity changes in this PR: the only non-script edits
are the dependency pin, its lock and the remappings it generates, so no
deployed bytecode and no deterministic address moves.
🤖 Generated with Claude Code
Summary by CodeRabbit