Correct checkCodeHash's stated justification in LibMigrationRegistry

src/lib/LibMigrationRegistry.sol

@@ -103,11 +103,13 @@ library LibMigrationRegistry {
     ///
     /// Verifies the registry's code hash before reading, so a chain where the
     /// registry is absent, or where something else occupies its address, is a
-    /// loud revert rather than a call into unknown code. That distinction is
-    /// the whole point here: "no registry on this chain" and "this migration
-    /// has not been applied" are different facts, and silently collapsing the
-    /// first into the second would send a caller down its pre-migration branch
-    /// on every chain the registry was never deployed to.
+    /// NAMED revert rather than a call into unknown code. An absent registry
+    /// reverts either way — solc reverts a high-level call whose returndata is
+    /// too short to decode — but anonymously, saying nothing about which of the
+    /// two it was. What the check actually forbids is the case that does NOT
+    /// revert: code at the address that is not this registry, an EIP-7702
+    /// delegation included, is free to answer zero to every migration and send
+    /// every caller down its pre-migration branch.
     ///
     /// The registry itself refuses the zero writer, and refuses the two ids a
     /// migration can never be, so those arrive as reverts from it rather than
@@ -129,10 +131,11 @@ library LibMigrationRegistry {
     /// if it has never applied one.
     ///
     /// Verifies the registry's code hash first for the same reason `applied`
-    /// does, and more sharply: a call into an empty account returns nothing,
-    /// which decodes as zero, and zero is the one value a head can never hold —
-    /// so an unverified read would hand back a head that is not a head at all,
-    /// on exactly the chains where nothing has been deployed.
+    /// does, and more sharply: occupying code is free to answer any head it
+    /// likes, including the zero no head can ever hold, so an unverified read
+    /// can hand back something that is not a head at all. An empty address is
+    /// not that case — there is no returndata for a `bytes32` to decode from,
+    /// so it reverts unguarded — and the check is what gives it a name.
     /// @param writer The namespace to read. Never the zero address.
     /// @return The head of `writer`'s namespace. Never zero.
     function head(address writer) internal view returns (bytes32) {
@@ -151,10 +154,12 @@ library LibMigrationRegistry {
     /// one account interleave into one chain.
     ///
     /// Verifies the registry's code hash before writing, so a migration is
-    /// never "applied" into an empty address or into unknown code. A record
-    /// that went nowhere is worse than no record at all: the migration would
-    /// have run, and every reader would go on asserting the pre-migration
-    /// state.
+    /// never "applied" into unknown code. A write into an EMPTY address fails
+    /// unguarded — solc checks the callee exists when no return data is
+    /// expected — so what this stops is the write that SUCCEEDS into something
+    /// that is not the registry: a record that went nowhere is worse than no
+    /// record at all, because the migration ran and every reader goes on
+    /// asserting the pre-migration state.
     ///
     /// The registry refuses the zero id, refuses a migration this caller has
     /// already applied, and refuses one applied onto anything but the

test/src/lib/LibMigrationRegistry.t.sol

@@ -262,11 +262,13 @@ contract LibMigrationRegistryTest is Test {
     }
 
     /// A chain with no registry deployed reverts on the code hash rather than
-    /// calling into an empty account. That call would succeed and return
-    /// nothing, which `abi.decode` would read as zero — "this migration has
-    /// not been applied", on every chain the registry was never deployed to,
-    /// which is exactly the silent pre-migration branch this library exists to
-    /// make impossible.
+    /// calling into an empty account, and the point of that is the NAME. An
+    /// empty account has no returndata for `abi.decode` to read, so the call
+    /// reverts unguarded too — anonymously, saying nothing about whether the
+    /// registry is absent or the migration unapplied. Those are different
+    /// facts, and this error is what tells them apart. The case that would
+    /// answer the lie instead of reverting is occupying code, covered by
+    /// `testAppliedWrongCode` and `testAppliedDelegatedCode`.
     function testAppliedNoRegistry(address writer, bytes32 migration) external {
         assertEq(LibMigrationRegistryDeploy.MIGRATION_REGISTRY_DEPLOYED_ADDRESS.code.length, 0);
 
@@ -280,10 +282,11 @@ contract LibMigrationRegistryTest is Test {
         this.externalApplied(writer, migration);
     }
 
-    /// Reading a head off a chain with no registry is refused for a sharper
-    /// version of the same reason: the empty-account read decodes as zero, and
-    /// zero is a value no head can ever hold, so an unverified read hands back
-    /// something that is not a head at all.
+    /// Reading a head off a chain with no registry is refused for the same
+    /// reason and with the same named error: an unguarded read reverts here
+    /// anyway, because there is no returndata for a `bytes32` to decode from,
+    /// so what the check adds is which chain state it was. A head that is not
+    /// a head is what occupying code can return, not what an empty one does.
     function testHeadNoRegistry(address writer) external {
         assertEq(LibMigrationRegistryDeploy.MIGRATION_REGISTRY_DEPLOYED_ADDRESS.code.length, 0);
 
@@ -297,10 +300,12 @@ contract LibMigrationRegistryTest is Test {
         this.externalHead(writer);
     }
 
-    /// Writing to a chain with no registry is refused for the mirror reason: an
-    /// `applyMigration` into an empty account is a migration that reports itself
-    /// applied and is not, which leaves every reader asserting the
-    /// pre-migration state forever.
+    /// Writing to a chain with no registry is refused by the code hash rather
+    /// than by solc's own existence check, which an `applyMigration` into an
+    /// empty account hits regardless — no return data is expected, so the
+    /// callee is checked to exist and the write reverts unnamed. A migration
+    /// that reports itself applied and is not is what a write into occupying
+    /// code does, and `testApplyMigrationWrongCode` is where that is covered.
     function testApplyMigrationNoRegistry(bytes32 expectedHead, bytes32 migration) external {
         assertEq(LibMigrationRegistryDeploy.MIGRATION_REGISTRY_DEPLOYED_ADDRESS.code.length, 0);