Repository: shazow/whatsabi (ts)

Issue:
proxies: contract incorrectly labeled as a proxy tampering the result
Note: I might be completely wrong and maybe it _is_ a proxy, but I'd rather try to explain what's happening naively so someone with better understanding can figure out.

## Overview

When fetching [this contract](https://etherscan.io/address/0xe1164a7a364929c3ba3da9671c8003dd71975d2d#code) (A) with `autoload` and `followProxies: true`, it is labeled as a proxy, and the function ends up following proxies up to [this contract](https://etherscan.io/address/0x03bd8b73d316521c0e65b5672e5be3f174b8a519#code) (B).

### Code

```ts
console.log("Loading contract", contractAddress);
const res = await autoload(contractAddress, {
  provider,
  abiLoader,
  followProxies: true,
  loadContractResult: true,
  onProgress: (p) => {
    console.log(p);
  },
  onError: (e) => {
    console.log(e);
  },
});
console.log("Result", res);
```

### Result

<img width="423" alt="Image" src="https://github.com/user-attachments/assets/c4dd2073-82f7-4de0-8442-9c1f0cc462e3" />

## Details

Setting `followProxies` to `false` produces the expected ABI and source for the provided contract address; which is a factory contract, and is labeled as such.

Contract B is the only proxy listed as proxies of contract A:
<img width="483" alt="Image" src="https://github.com/user-attachments/assets/698d37da-1c4f-458d-b3a4-3b19ee1ce822" />

The contract does not appear to be a proxy according to Etherscan:
<img width="858" alt="Image" src="https://github.com/user-attachments/assets/daf48980-67d6-4c9b-8600-699b768cba5c" />

All of this is ran in the `EtherscanLoader` (as the contract is not verified on Sourcify).

## Links

- [Contract A (fetched) deployment tx](https://etherscan.io/tx/0xaecfdc5c0a46f75ceaca20fc51949993e1f3d5502487a2d43302af6077cf2266)
- [Contract B (resolved) deployment tx](https://etherscan.io/tx/0xd857dcc116905ce491feda2b54f4900f784db61a8a4221be0e39a72b1ee86ce6)

## How to run

```ts
const res = await autoload(contractAddress, {
  provider,
  abiLoader: new loaders.MultiABILoader([
    new loaders.SourcifyABILoader({ chainId: chain.id }),
    new loaders.EtherscanABILoader({
      baseURL: 'https://api.etherscan.io/api',
      apiKey: env.ETHERSCAN_API_KEY,
    }),
  ]),
  followProxies: false, // true will directly follow up to contract B
  loadContractResult: true,
});

console.log('Result', res);

const proxies = await res.followProxies?.();
console.log({ resolvedProxies: proxies }); // will point to contract B
```

Pull request:
proxies: Skip followProxies if contract is complex enough to be a destination itself
Fixes #173 

Summary: Sometimes a contract contains proxy-like code, but it's actually a destination contract itself.

This PR adds a heuristic where we skip the `AutoloadResult.followProxies` when the bytecode contains more than 4 SSTORE instructions.

I sampled a bunch of proxies, and most upgradeable proxies have 1-2 sstores, and typical contracts have dozens of sstores. 3 felt a little tight, but 5 feels pretty safe. Can tweak this later.

Patch:
diff --git a/examples/autoload.ts b/examples/autoload.ts
index 9f0c14e..6aac6b9 100755
--- a/examples/autoload.ts
+++ b/examples/autoload.ts
@@ -34,7 +34,7 @@ async function main() {
         process.exit(1);
     }
 
-    let extraConfig : AutoloadConfig = whatsabi.loaders.defaultsWithEnv(env);
+    let extraConfig : object = whatsabi.loaders.defaultsWithEnv(env);
     if (env.SKIP_LOOKUPS) {
         console.debug("Skipping lookups, only using bytecode");
         extraConfig = {
@@ -59,7 +59,12 @@ async function main() {
         const detectedInterfaces = whatsabi.interfaces.abiToInterfaces(abi);
         if (detectedInterfaces.length) console.log("detected interfaces:", detectedInterfaces);
 
-        if (!r.followProxies) break;
+        if (!r.followProxies) {
+            if (r.proxies.length) {
+                console.log("proxies detected but not following:", r.proxies);
+            }
+            break;
+        }
 
         console.log("following proxies...");
         r = await r.followProxies();
diff --git a/src/auto.ts b/src/auto.ts
index 6507fc6..ac9cc69 100644
--- a/src/auto.ts
+++ b/src/auto.ts
@@ -12,6 +12,11 @@ import { defaultABILoader, defaultSignatureLookup } from "./loaders.js";
 import { abiFromBytecode, disasm } from "./disasm.js";
 import { MultiABILoader } from "./loaders.js";
 
+
+/// Magic number we use to determine whether a proxy is reasonably a destination contract or not.
+/// If it has many SSTORE's then it's probably doing something other than proxying.
+const PROXY_SSTORE_COUNT_MAX = 4;
+
 function isAddress(address: string) {
     return address.length === 42 && address.startsWith("0x") && Number(address) >= 0;
 }
@@ -91,8 +96,11 @@ export type AutoloadConfig = {
     /** Settings: */
 
     /**
-     * Enable following proxies automagically, if possible. Return the final result.
-     * Note that some proxies are relative to a specific selector (such as DiamondProxies), so they will not be followed
+     * Enable following proxies automagically if *reasonable*. Return the final result.
+     *
+     * Some caveats:
+     * - Proxies that are relative to a specific selector (such as DiamondProxies) will not be followed.
+     * - Contracts that are not primarily proxies will not be followed. Current heuristic is containing at most 5 SSTORE instructions. (See Issue #173)
      *
      * @group Settings
      */
@@ -216,7 +224,7 @@ export async function autoload(address: string, config: AutoloadConfig): Promise
         const f = await diamondProxy.facets(provider, address);
         Object.assign(facets, f);
 
-    } else if (result.proxies.length > 0) {
+    } else if (result.proxies.length > 0 && program.sstoreCount <= PROXY_SSTORE_COUNT_MAX) {
         result.followProxies = async function(selector?: string): Promise<AutoloadResult> {
             // This attempts to follow the first proxy that resolves successfully.
             // FIXME: If there are multiple proxies, should we attempt to merge them somehow?
diff --git a/src/disasm.ts b/src/disasm.ts
index 541d67b..3e0ca03 100644
--- a/src/disasm.ts
+++ b/src/disasm.ts
@@ -154,6 +154,7 @@ export class Program {
     proxySlots: Array<string>; // PUSH32 found that match known proxy slots
     proxies: Array<ProxyResolver>;
     isFactory: boolean; // CREATE or CREATE2 detected
+    sstoreCount: number; // Number of SSTORE instructions detected, used to determine if this may be a destination contract (vs a proxy)
 
     init?: Program; // Program embedded as init code
 
@@ -165,6 +166,7 @@ export class Program {
         this.proxySlots = [];
         this.proxies = [];
         this.isFactory = false;
+        this.sstoreCount = 0;
         this.init = init;
     }
 }
@@ -368,6 +370,8 @@ export function disasm(bytecode: string, config?: {onlyJumpTable: boolean}): Pro
                 // CREATE/CREATE2 are part of interestingOpCodes so we can leverage this short circuit
                 if (inst === opcodes.CREATE || inst === opcodes.CREATE2) {
                     p.isFactory = true;
+                } else if (inst === opcodes.SSTORE) {
+                    p.sstoreCount += 1;
                 }
             }
         }


Test patch:
diff --git a/src/__tests__/auto.test.ts b/src/__tests__/auto.test.ts
index acf3d4e..e929d42 100644
--- a/src/__tests__/auto.test.ts
+++ b/src/__tests__/auto.test.ts
@@ -1,4 +1,4 @@
-import { expect } from 'vitest';
+import { expect } from "vitest";
 
 import { whatsabi } from "../index";
 import { autoload } from "../auto";
@@ -9,12 +9,12 @@ const TIMEOUT = 15000;
 
 test('autoload throws typed error', async () => {
     // @ts-expect-error: Expected 2 arguments, but got 1
-    await expect(autoload("0xf00")).rejects.toThrow(/config is undefined/);
+    await expect(autoload("0xf00")).rejects.toThrow(new RegExp("config is undefined"));
 
     const fakeProvider = {
         request: () => { },
-    }
-    await expect(autoload("abc.eth", { provider: fakeProvider })).rejects.toThrow(/Failed to resolve ENS/);
+    };
+    await expect(autoload("abc.eth", { provider: fakeProvider })).rejects.toThrow(new RegExp("Failed to resolve ENS"));
 });
 
 test('autoload sets hasCode to false if code is empty', async () => {
@@ -113,7 +113,7 @@ online_test('autoload loadContractResult verified etherscan', async ({ provider,
     expect(result.contractResult?.name).toBe("TransparentUpgradeableProxy");
     expect(result.contractResult?.compilerVersion).toBe("v0.8.15+commit.e14f2714");
     expect(result.contractResult?.loaderResult?.Proxy).toBe("1");
-    expect(result.contractResult?.loaderResult?.Implementation).toMatch(/^0x[0-9a-f]{40}$/);
+    expect(result.contractResult?.loaderResult?.Implementation).toMatch(new RegExp("^0x[0-9a-f]{40}$"));
 });
 
 cached_test('autoload isFactory', async ({ provider, env, withCache }) => {
@@ -126,9 +126,31 @@ cached_test('autoload isFactory', async ({ provider, env, withCache }) => {
         },
     )
     const result = await autoload(address, {
-        provider: provider,
+        provider: whatsabi.providers.WithCachedCode(provider, {
+            [address]: code,
+        }),
         ...whatsabi.loaders.defaultsWithEnv(env),
     });
     expect(result.isFactory).toBeTruthy();
 });
 
+cached_test('autoload ambiguous proxy', async ({ provider, env, withCache }) => {
+    // Issue #173
+    const address = "0xe1164a7a364929c3ba3da9671c8003dd71975d2d";
+
+    const code = await withCache(
+        `${address}_code`,
+        async () => {
+            return await provider.getCode(address)
+        },
+    )
+    const result = await autoload(address, {
+        provider: whatsabi.providers.WithCachedCode(provider, {
+            [address]: code,
+        }),
+        ...whatsabi.loaders.defaultsWithEnv(env),
+    });
+    expect(result.proxies.length).toBe(1);
+    expect(result.followProxies).toBeFalsy();
+});
+