Repository: shazow/whatsabi
Pull request #182 (2025-05-14 15:42:10)

proxies: DiamondProxy should override off-by-one DIAMOND_STORAGE slot too
Add facet tests to comprehensive test suite

Closes https://github.com/shazow/whatsabi/pull/181


Patch:
diff --git a/src/proxies.ts b/src/proxies.ts
index 5a579b6..5eb4477 100644
--- a/src/proxies.ts
+++ b/src/proxies.ts
@@ -62,7 +62,6 @@ export interface ProxyResolver {
 
 // Some helpers:
 
-
 const _zeroAddress = "0x0000000000000000000000000000000000000000";
 
 // Convert 32 byte hex to a 20 byte hex address
@@ -70,13 +69,6 @@ function addressFromPadded(data:string): string {
     return "0x" + data.slice(data.length - 40);
 }
 
-// Given a slot, return the +1 version because some contracts use the off-by-one version
-// Example: bbbb -> bbbc
-// Warning: No error checking! Won't work if final letter is "a", garbage in garbage out.
-// This basically exists as an internal helper to cut down on code.
-function slotOffByOne(slot: string, offset: number = 1): string {
-    return slot.slice(0, -1) + String.fromCharCode(slot.charCodeAt(slot.length - 1) + offset);
-}
 
 // Resolvers:
 
@@ -158,6 +150,12 @@ const diamondSelectors = [
 // ERC2535 - Diamond/Facet Proxy
 export class DiamondProxyResolver extends BaseProxyResolver implements ProxyResolver {
     override name = "DiamondProxy";
+    readonly storageSlot : string;
+
+    constructor(name: string, storageSlot: string) {
+        super(name);
+        this.storageSlot = storageSlot ?? slots.DIAMOND_STORAGE;
+    }
 
     async resolve(provider: StorageProvider & CallProvider, address: string, selector: string): Promise<string> {
         if (!selector) {
@@ -170,7 +168,7 @@ export class DiamondProxyResolver extends BaseProxyResolver implements ProxyReso
         //
         // ethers.utils.defaultAbiCoder.encode(["bytes4", "bytes32"], ["0x" + selector, slots.DIAMOND_STORAGE])
         // keccak256("0x" + selector.padEnd(64, "0") + slots.DIAMOND_STORAGE.slice(2));
-        const facetMappingSlot = joinSlot([selector.padEnd(64, "0"), slots.DIAMOND_STORAGE]);
+        const facetMappingSlot = joinSlot([selector.padEnd(64, "0"), this.storageSlot]);
 
         const facet = await provider.getStorageAt(address, facetMappingSlot);
 
@@ -198,7 +196,10 @@ export class DiamondProxyResolver extends BaseProxyResolver implements ProxyReso
 
     // Return the facet-to-selectors mapping
     // Note that this does not respect frozen facet state.
-    async facets(provider: StorageProvider, address: string): Promise<Record<string, string[]>> {
+    async facets(provider: StorageProvider, address: string, config: { limit: number }): Promise<Record<string, string[]>> {
+        // limit is used to get partial results, mainly for testing
+        let limit = config?.limit || 0;
+
         // Would be cool if we could read the private facets storage and return known selectors... let's do it!
         //
         // Shoutout to @banteg for sharing the rest of the owl:
@@ -222,7 +223,7 @@ export class DiamondProxyResolver extends BaseProxyResolver implements ProxyReso
         //   address[] facets;
         //   bool isFrozen;
         // }
-        const storageStart = slots.DIAMOND_STORAGE;
+        const storageStart = this.storageSlot;
 
         const facetsOffset = addSlotOffset(storageStart, 2); // Facets live in the 3rd slot (0-indexed)
         const addressWidth = 20; // Addresses are 20 bytes
@@ -243,6 +244,8 @@ export class DiamondProxyResolver extends BaseProxyResolver implements ProxyReso
             const facetSelectorsSlot = joinSlot([facet, slot]);
             const selectors = await readArray(provider, address, facetSelectorsSlot, selectorWidth);
             facetSelectors[addressWithChecksum(facet)] = selectors.map(s => "0x" + s);
+
+            if (--limit === 0) break;
         }
 
         return facetSelectors;
@@ -338,6 +341,9 @@ export const slots : Record<string, string> = {
     // keccak256("diamond.standard.diamond.storage") - 1;
     DIAMOND_STORAGE: "0xc8fcad8db84d3cc18b4c41d551ea0ee66dd599cde068d998e57d5e09332c131b",
 
+    // Same as above but some implementations don't do -1
+    DIAMOND_STORAGE_1: "0xc8fcad8db84d3cc18b4c41d551ea0ee66dd599cde068d998e57d5e09332c131c",
+
     // EIP-1167 minimal proxy standard
     // Parsed in disasm
 }
@@ -350,12 +356,12 @@ export const slotResolvers : Record<string, ProxyResolver> = {
     [slots.PROXIABLE]: new PROXIABLEProxyResolver("PROXIABLE"),
     [slots.GNOSIS_SAFE_SELECTOR]: new GnosisSafeProxyResolver("GnosisSafeProxy"),
     [slots.DIAMOND_STORAGE]: new DiamondProxyResolver("DiamondProxy"),
+    [slots.DIAMOND_STORAGE_1]: new DiamondProxyResolver("DiamondProxy", slots.DIAMOND_STORAGE_1),
 
     // Not sure why, there's a compiler optimization that adds 1 or 2 to the normal slot?
     // Would love to understand this, if people have ideas
-    [slotOffByOne(slots.DIAMOND_STORAGE, 1)]: new DiamondProxyResolver("DiamondProxy"),
-    [slotOffByOne(slots.DIAMOND_STORAGE, 2)]: new DiamondProxyResolver("DiamondProxy"),
+    "0xc8fcad8db84d3cc18b4c41d551ea0ee66dd599cde068d998e57d5e09332c131d": new DiamondProxyResolver("DiamondProxy"),
 
     // Off-by-one slot version of EIP1967, some examples in the wild who choose to do the -1 at runtime (See issue #178)
-    [slotOffByOne(slots.EIP1967_IMPL, 1)]: new EIP1967ProxyResolver("EIP1967Proxy"),
+    "0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbd": new EIP1967ProxyResolver("EIP1967Proxy"),
 };


Test patch:
diff --git a/src/__tests__/env.ts b/src/__tests__/env.ts
index 1e139af..684442b 100644
--- a/src/__tests__/env.ts
+++ b/src/__tests__/env.ts
@@ -13,6 +13,7 @@ const env = {
 
     PROVIDER: process.env.PROVIDER,
     PROVIDER_RPC_URL: process.env.PROVIDER_RPC_URL,
+    ETH_RPC_URL: process.env.ETH_RPC_URL, // Override for all
 };
 
 const DEFAULT_PUBLIC_RPC = "https://ethereum-rpc.publicnode.com";
@@ -24,7 +25,9 @@ const DEFAULT_PUBLIC_RPC = "https://ethereum-rpc.publicnode.com";
 export function makeProvider(rpc_url?: string): Provider {
     return CompatibleProvider(function() {
         if (!rpc_url) {
-            if (env.INFURA_API_KEY) {
+            if (env.ETH_RPC_URL) {
+                rpc_url = env.ETH_RPC_URL;
+            } else if (env.INFURA_API_KEY) {
                 rpc_url = "https://mainnet.infura.io/v3/" + env.INFURA_API_KEY;
             } else {
                 rpc_url = env.PROVIDER_RPC_URL;
diff --git a/src/__tests__/proxies.test.ts b/src/__tests__/proxies.test.ts
index d93b632..eaf328a 100644
--- a/src/__tests__/proxies.test.ts
+++ b/src/__tests__/proxies.test.ts
@@ -264,6 +264,9 @@ describe('comprehensive proxy detection', () => {
             expect(program.proxies.length).toEqual(1);
             const resolver = program.proxies[0];
             expect(resolver.name).toEqual("DiamondProxy");
+
+            const facets = await (resolver as proxies.DiamondProxyResolver).facets(provider, address, { limit: 1 });
+            expect(facets).to.not.be.empty;
         });
     });
 
@@ -282,6 +285,13 @@ describe('comprehensive proxy detection', () => {
         expect(program.proxies.length).toEqual(1);
         const resolver = program.proxies[0];
         expect(resolver.name).toEqual("DiamondProxy");
+
+        const selector = "0x736eac0b";
+        const got = await resolver.resolve(provider, address, selector);
+        expect(got).not.toEqual("0x0000000000000000000000000000000000000000");
+
+        const facets = await (resolver as proxies.DiamondProxyResolver).facets(provider, address, { limit: 1 });
+        expect(facets).to.not.be.empty;
     });
 
 });