AddressRegistry: the write-once IAddressRegistryV1 concrete - #7
AddressRegistry: the write-once IAddressRegistryV1 concrete#7thedavidmeister wants to merge 2 commits into
Conversation
…ncrete An immutable root authority binds an opaque `bytes32` name to an address, once; nothing, root included, can change one after; and reading an unbound name reverts rather than answering with the zero address, so no caller has to remember to check. That is the whole contract. No rotation, no removal, no upgrade, no admin surface and no second reader — each of those turns a binding from a constant back into a value that can move, which is the one property the registry exists to provide, and the property that makes a deploy-time check of a binding worth anything. `ADDRESS_REGISTRY_ROOT` is a PLACEHOLDER. The root is a constant in the creation code, so it is part of the contract's identity: changing it moves the deterministic deploy address and code hash on every network. No deploy-pin snapshot, deploy suite or pin lib is generated here for that reason, and `AddressRegistryDeployPinsTest` fails if this source and `rain-deploy`'s `LibAddressRegistry` pins ever disagree. The interface comes from `rain-deploy`, which the bump from 0.1.3 to 0.1.6 brings in. That bump also drops `deployAndBroadcast`'s trailing dependency code-hash mapping, which no longer exists upstream.
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. WalkthroughAdded ChangesAddressRegistry implementation and deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CLAUDE.md`:
- Around line 8-21: The deployment documentation must distinguish approved
production artifacts from test deployments and external rain-deploy pins. In
CLAUDE.md lines 8-21, state that this repository has no approved local
production snapshot yet, while LibAddressRegistry pins and pin tests already
exist. In CLAUDE.md lines 123-128, state that AddressRegistry is excluded from
production script/Deploy.sol and has no approved local generated snapshot; do
not characterize it as lacking deployment or pin support entirely.
In `@foundry.toml`:
- Line 37: Revert the rain-deploy version in foundry.toml to the released 0.1.3
version and keep soldeer.lock consistent; do not use 0.1.6 until it is
officially published.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 5a3617f4-e1fc-4ee3-9540-3296f7f04694
📒 Files selected for processing (13)
.gas-snapshotCLAUDE.mdREADME.mdfoundry.tomlscript/BuildPointers.solscript/Deploy.solsrc/concrete/AddressRegistry.soltest/src/concrete/AddressRegistryDeployPins.t.soltest/src/concrete/AddressRegistryGet.t.soltest/src/concrete/AddressRegistryRegister.t.soltest/src/lib/LibCloneFactoryDeploy.t.soltest/src/lib/LibCloneFactoryDeployProd.t.soltest/src/lib/LibCloneFactoryDeployTaggedConstants.t.sol
…repo Said it had 'no deploy pins' and 'no pin lib', which is wrong: rain-deploy's LibAddressRegistry pins its address and code hash, and AddressRegistryDeployPins tests them here. What it has none of is a src/generated/<tag>/ snapshot and a Deploy.sol suite, and the reason those must wait is that the directory is append-only, so a placeholder-derived snapshot could never be corrected.
|
Superseded by rainlanguage/rain.deploy#26. Leaving this open rather than closing it — that call is the human's.
Two problems this PR had are removed by the move rather than reasoned around:
The one thing here that does not carry over and may be worth keeping is the |
|
🤖 ai:producer Reproduced verbatim in this checkout's own CI toolchain (nix develop github:rainlanguage/rainix/53e96a7d0a97d7c7c75c3b2412521324776fdac6#sol-shell -c forge soldeer install): Error: Failed to run soldeer: error during remappings operation: dependency not found: rain-deploy~0.1.6 Ground truth, read off the soldeer registry rather than the PR prose: the latest published rain-deploy is 0.1.5 (2026-07-30). 0.1.6 does not exist. foundry.toml pins rain-deploy = 0.1.6; soldeer.lock still carries 0.1.3. No code fix exists on this branch. A downgrade to 0.1.5 is not available: rain.deploy at tag sol-v0.1.5 AND at main today ships only src/lib/LibRainDeploy.sol -- there is no IAddressRegistryV1 and no LibAddressRegistry in any published version. Both symbols exist only on rain.deploy#26's branch, which is still OPEN and unmerged. Vendoring or re-declaring the interface locally is the design this split exists to prevent, so it is not a repair either. This is NOT the deploy-pin / migration shape despite the .deploy repo: no prod-pin, testProdDeploy* or deploy-pinned-constant check is red. AddressRegistryDeployPins.t.sol never runs at all. Nothing here is deployable and no deploy is wanted. This is also NOT environmental: the registry is up and answered correctly; it simply does not hold the requested version. Clearance: merging rain.deploy#26 autopublishes rain-deploy 0.1.6. Then SECOND, INDEPENDENT GATE once the above clears -- human judgement, not machine work: ADDRESS_REGISTRY_ROOT in src/concrete/AddressRegistry.sol is the placeholder 0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD. It is a constant in the creation code, so it is part of the contract's deterministic identity on every network; a human must supply the real root, and rain-deploy's LibAddressRegistry pins must be re-derived in that same change. |
1 participant
The concrete half of
rainlanguage/rain.deploy#25. The interface, the
reader library and the cross-network deploy gate are in
rainlanguage/rain.deploy#26; this is the contract that
implements the interface. Merging both completes the issue.
Blocked on rain.deploy#26
This imports
IAddressRegistryV1fromrain-deploy0.1.6, which is notpublished yet —
rain.deploy's[package].versionis the next, unpublishedversion, and merging
rainlanguage/rain.deploy#26 is what autopublishes it
(latest on the registry today is 0.1.5).
So until that merges:
forge soldeer install, because the version does not exist.soldeer.lockstill carriesrain-deploy 0.1.3; its 0.1.6 entry needs thepublished artifact's checksum, which cannot be fabricated. Once 0.1.6 is on
the registry,
forge soldeer installregenerates it and the result wantscommitting here.
The interface is deliberately not vendored or re-declared locally to dodge
that: the whole point of the split is that one declaration lives in the library
repo and the concrete implements it.
If the merge of rain.deploy#26 publishes something other than 0.1.6 (i.e. if
another PR lands there first), the version pin and the four import prefixes here
move with it.
The root authority is a placeholder — a human must supply it
ADDRESS_REGISTRY_ROOTinsrc/concrete/AddressRegistry.solis0xdeaDDeADDEaDdeaDdEAddEADDEAdDeadDEADDEaD. I do not have the real root andwill not invent one.
The root is a constant in the creation code, so it is part of the contract's
identity: changing it changes the deterministic Zoltu address and the code hash
on every network. Consequently:
src/generated/<tag>/AddressRegistry.pointers.sol, no pin lib, noDeploy.solsuite. Generating one now would freeze an address nobody can use,into a directory that CI enforces as append-only.
rain-deploy'sLibAddressRegistry.ADDRESS_REGISTRY/ADDRESS_REGISTRY_CODEHASHare derived from the placeholder build and must bere-derived in the same change that supplies the real root.
AddressRegistryDeployPinsTesthere fails the moment those two disagree, sothe two repos cannot drift apart silently. Changing only the root and pushing
turns this repo red until the pins follow — that is the intended forcing
function, demonstrated as a mutation below.
The contract
An immutable root binds an opaque
bytes32name to an address, once; nothing,root included, can change one after; reading an unbound name reverts. Two
functions, four errors, one event, and nothing else — no rotation, no removal,
no upgrade, no pause, no admin surface, and no second reader.
Two details that are not decoration:
registerrejects the zero address. An unbound name reads as zerointernally, so binding zero would produce a name that is both bound and
unreadable — and that
registerwould accept a second time. Without this,write-once is violable.
internal, notpublic. A public mapping'sgenerated getter answers an unbound name with the zero address, which is
exactly the silent failure the reverting
getexists to prevent.Also in this PR, and why
longer describe it as only "the deployment half of
rain.factory". TheGitHub repo description still says that and wants updating too — I cannot
change it. Suggested: "Rain's Zoltu-deployed concrete contracts and their
deploy pins."
rain-deploy0.1.3 → 0.1.6. Required — soldeer resolves one version perpackage, so the interface has to come from the same version
LibRainDeploydoes. The bump also drops
deployAndBroadcast's trailing dependencycode-hash mapping, which no longer exists upstream, so
script/Deploy.solloses that argument and its now-unused
sDepCodeHashesfield. Behaviour forCloneFactoryis unchanged: it passed an empty dependency list already..gas-snapshotregenerated for the new tests.QA
Discriminating tests
test/src/concrete/AddressRegistryRegister.t.sol(8) —testRegisterOnlyRoot,testRegisterOnlyRootWhenAlreadyBound(authority ischecked even when the name is already bound),
testRegisterWriteOnce,testRegisterWriteOnceSameAccount(a no-op rebind is not a special case thatslips through),
testRegisterZeroAccount(and that the name stays unboundafter),
testRegisterDistinctNames,testRegisterEvent(topics and empty data,so the indexing is pinned),
testRegisterNoEventOnRevert.test/src/concrete/AddressRegistryGet.t.sol(5) —testGetUnsetReverts,testGetReturnsRegistered(twice, so reading does not consume),testGetOpaqueNames(bytes32(0),1,type(uint256).max— names astring-hashing convention would never produce),
testGetNoGeneratedMappingGetter,testGetNoOtherEntryPoint(no fallback, noreceive, nothing outside the two interface selectors).
test/src/concrete/AddressRegistryDeployPins.t.sol(2) — the cross-repo gate:the address
LibRainDeploy.zoltuAddressderives from this source and the codehash of its runtime code MUST equal
rain-deploy's pins, and actually deployingthe creation code through the etched Zoltu factory MUST land there.
Mutations applied, and what killed each
Applied to the committed tree, one at a time, whole suite run, tree restored.
Baseline green. Killers below list only the behavioural test — every mutation
also trips the two pin tests, because any source change moves the bytecode,
which is itself the point of those two.
register: root check removedtestRegisterOnlyRoot,testRegisterOnlyRootWhenAlreadyBound,testRegisterNoEventOnRevertregister: root check inverted (!=→==)testGetReturnsRegistered,testGetOpaqueNamesregister: zero-address check removedtestRegisterZeroAccountregister: write-once check removedtestRegisterWriteOnce,testRegisterWriteOnceSameAccountregister: write-once carve-out so an identical rebind is allowedtestRegisterWriteOnce,testRegisterWriteOnceSameAccountregister:Registernot emittedtestRegisterEventregister:Registeremitted withmsg.senderinstead ofaccounttestRegisterEventget: unbound name returns zero instead of revertingtestGetUnsetReverts,testRegisterDistinctNames,testRegisterZeroAccountpublictestGetNoGeneratedMappingGetter,testGetNoOtherEntryPointtestAddressRegistryPinsDeriveFromThisSource,testAddressRegistryDeploysToPinnedAddressNo survivors.
Oracle
The issue is the oracle, not the code: immutable root, write-once
bytes32 => address, revert on unset, nothing else. The pins have anindependent second oracle — the Zoltu factory itself, deployed to and read back,
rather than a recomputation of the same formula the library uses.
Category check
The issue's motivating example is initial ownership, but it says the problem is
not owner-specific, and nothing here is: the key is an opaque
bytes32, thevalue is an address, and the contract has no notion of an owner, a role or an
initializer. The "no second reader" requirement is covered as a property (no
entry point outside the two interface selectors) rather than only as the one
named instance (the generated mapping getter). Every error the contract can
revert with has a test, and every check has a test for the case where it is the
first check to fire.
n/a
rain-deploy0.1.6, as described above. Locally, with 0.1.6 materialised independencies/(gitignored) from that PR's branch, all 30 non-fork tests pass,forge fmt --check,slither .(0 findings),reuse lintandrainix-sol-single-contractare clean. The fiveLibCloneFactoryDeployProdTestfork tests need the RPC secrets CI has.
AddressRegistry— n/a, it cannot be deployed untilthe real root is supplied.
Summary by CodeRabbit
New Features
Documentation
Tests