Skip to content

Conversation

gonzaotc
Copy link
Contributor

@gonzaotc gonzaotc commented Aug 13, 2025

Pending:

v0.0.0.alpha

  • Make the Pausable option to pause common entry points and custom external/public non-view functions by default.
  • When enabling extra permissions, add the respective permission hook function to the output.
  • Make shares required in BaseCustomAccounting & BaseCustomCurve
  • Simplify Hook names
  • Provide minimal (working) hook implementations, rather than leaving open to implementation.
  • Simplify imports
  • Add tests covering compilation and snapshots on every option and multiple hook permissions config
  • Update Uniswap Hooks project with latest changes to point to master instead of specific commit

Next release in a posterior PR:

  • Fix full inheritance linearization by refactoring BaseHook and adding respective .super calls in inheriting hooks contracts.
  • Enable multiple hook selection.

@gonzaotc gonzaotc requested review from a team as code owners August 13, 2025 00:20
@gonzaotc gonzaotc marked this pull request as draft August 13, 2025 00:20
Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

github-actions bot commented Aug 13, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link

PR Type

Enhancement


Description

  • Add Uniswap Hooks wizard with comprehensive contract generation

  • Implement using directive support in Solidity contract builder

  • Create new UI interface for Uniswap Hooks configuration

  • Add navigation and routing for new wizard type


Diagram Walkthrough

flowchart LR
  A["Core Solidity"] --> B["Using Directives"]
  C["Uniswap Hooks Core"] --> D["Hook Types"]
  C --> E["Contract Builder"]
  F["UI Components"] --> G["Hooks Controls"]
  F --> H["Navigation"]
  I["Public Pages"] --> J["Routing"]
  D --> K["Generated Contract"]
  E --> K
Loading

File Walkthrough

Relevant files
Enhancement
20 files
contract.ts
Add using directive support to contract builder                   
+10/-0   
print.ts
Implement using directive printing functionality                 
+18/-0   
api.ts
Create main API interface for hooks wizard                             
+16/-0   
build-generic.ts
Implement generic contract builder for hooks                         
+16/-0   
hooks.ts
Add hooks options generation utilities                                     
+31/-0   
hooks.ts
Core hooks contract building and configuration logic         
+230/-0 
index.ts
Main export file for hooks package                                             
+12/-0   
kind.ts
Kind validation and sanitization utilities                             
+26/-0   
languages-types.ts
Add uniswap-hooks language type support                                   
+4/-2     
main.ts
Integrate uniswap-hooks app routing and initialization     
+13/-1   
inject-hyperlinks.ts
Add hyperlink injection for contract imports                         
+18/-0   
remix.ts
Implement Remix IDE integration utilities                               
+15/-0   
AccessControlSection.svelte
Create access control configuration component                       
+62/-0   
App.svelte
Main application component for hooks wizard                           
+264/-0 
HooksControls.svelte
Comprehensive hooks configuration controls interface         
+172/-0 
InfoSection.svelte
Contract information configuration section                             
+32/-0   
cairo.html
Add uniswap-hooks navigation link                                               
+1/-0     
index.html
Add uniswap-hooks navigation link                                               
+1/-0     
stellar.html
Add uniswap-hooks navigation link                                               
+1/-0     
stylus.html
Add uniswap-hooks navigation link                                               
+1/-0     
Configuration changes
2 files
version.ts
Version compatibility configuration                                           
+4/-0     
highlightjs.ts
Configure syntax highlighting for hooks                                   
+7/-0     
Additional files
11 files
package.json +1/-0     
CHANGELOG.md +6/-0     
LICENSE +661/-0 
NOTICE +8/-0     
README.md +1/-0     
ava.config.js +9/-0     
package.json +36/-0   
tsconfig.json +16/-0   
uniswap-hooks.html +87/-0   
vars.css +2/-0     
standalone.css +4/-0     

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The new using-handling (addUsing) is added in the builder and printing, but the Contract interface only shows using: Using[] without any new type definition in this diff. Verify that the Using type and initialization of this.using are correctly declared/initialized to avoid runtime errors and type mismatches, and ensure addUsing is wired in ContractBuilder constructor.

  using: Using[];
  imports: ImportContract[];
  functions: ContractFunction[];
  constructorCode: string[];
  constructorArgs: FunctionArgument[];
  variables: string[];
  upgradeable: boolean;
}

export type Value = string | number | { lit: string } | { note: string; value: Value };

export interface Parent {
  contract: ImportContract;
  params: Value[];
  importOnly?: boolean;
}

export interface ImportContract extends ReferencedContract {
  path: string;
}

export interface ReferencedContract {
  name: string;
  transpiled?: boolean;
}

export interface Using {
  library: ImportContract;
  usingFor: string;
}

export interface BaseFunction {
  name: string;
  args: FunctionArgument[];
  returns?: string[];
  kind: FunctionKind;
  mutability?: FunctionMutability;
}

export interface ContractFunction extends BaseFunction {
  override: Set<ReferencedContract>;
  modifiers: string[];
  code: string[];
  mutability: FunctionMutability;
  final: boolean;
  comments: string[];
}

export type FunctionKind = 'internal' | 'public';
export type FunctionMutability = (typeof mutabilityRank)[number];

// Order is important
const mutabilityRank = ['pure', 'view', 'nonpayable', 'payable'] as const;

function maxMutability(a: FunctionMutability, b: FunctionMutability): FunctionMutability {
  return mutabilityRank[Math.max(mutabilityRank.indexOf(a), mutabilityRank.indexOf(b))]!;
}

export interface FunctionArgument {
  type: string | ReferencedContract;
  name: string;
}

export interface NatspecTag {
  key: string;
  value: string;
}

export class ContractBuilder implements Contract {
  readonly name: string;
  license: string = 'MIT';
  upgradeable = false;

  readonly using: Using[] = [];
  readonly natspecTags: NatspecTag[] = [];

  readonly constructorArgs: FunctionArgument[] = [];
  readonly constructorCode: string[] = [];
  readonly variableSet: Set<string> = new Set();

  private parentMap: Map<string, Parent> = new Map<string, Parent>();
  private functionMap: Map<string, ContractFunction> = new Map();

  constructor(name: string) {
    this.name = toIdentifier(name, true);
  }

  get parents(): Parent[] {
    return [...this.parentMap.values()]
      .filter(p => !p.importOnly)
      .sort((a, b) => {
        if (a.contract.name === 'Initializable') {
          return -1;
        } else if (b.contract.name === 'Initializable') {
          return 1;
        } else {
          return 0;
        }
      });
  }

  get imports(): ImportContract[] {
    return [...[...this.parentMap.values()].map(p => p.contract), ...this.using.map(u => u.library)];
  }

  get functions(): ContractFunction[] {
    return [...this.functionMap.values()];
  }

  get variables(): string[] {
    return [...this.variableSet];
  }

  addParent(contract: ImportContract, params: Value[] = []): boolean {
    const present = this.parentMap.has(contract.name);
    this.parentMap.set(contract.name, { contract, params });
    return !present;
  }

  addImportOnly(contract: ImportContract): boolean {
    const present = this.parentMap.has(contract.name);
    this.parentMap.set(contract.name, {
      contract,
      params: [],
      importOnly: true,
    });
    return !present;
  }

  addUsing(library: ImportContract, usingFor: string): boolean {
    const exists = this.using.some(u => u.library.name === library.name && u.usingFor === usingFor);
    if (!exists) {
      this.using.push({ library, usingFor });
      return true;
    }
    return false;
  }
Logic Error

sanitizeKind lowercases and uppercases only the first character, which turns most strings into a single capital letter (e.g., "uniswap-hooks" -> "U"), then isKind only accepts "Hooks". This effectively collapses any input into "Hooks" except the exact "Hooks". Confirm intended behavior or adjust to properly normalize full strings.

export function sanitizeKind(kind: unknown): Kind {
  if (typeof kind === 'string') {
    const sanitized = kind.trim().toLowerCase().charAt(0).toUpperCase();
    if (isKind(sanitized)) {
      return sanitized;
    }
  }
  return 'Hooks';
}

function isKind<T>(value: Kind | T): value is Kind {
  switch (value) {
    case 'Hooks':
      return true;

    default: {
      // Static assert that we've checked all kinds.
      const _: T = value;
      return false;
    }
  }
}
Incorrect Docs

Help tooltips for several options are placeholders or incorrect (e.g., ERC6909 described as non-fungible, Pausable text repeated across various unrelated features). Update copy and links to accurate docs to avoid user confusion.

  <div class="checkbox-group">
    {#each ALL_HOOKS.slice(0, 4) as hookName}
        <label class:checked={opts.hook === hookName}>
          <input type="radio" bind:group={opts.hook} value={hookName} />
          {hookName}
          <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
            @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
            emergency response.
          </HelpTooltip>
        </label>
    {/each}
  </div>

</section>

<section class="controls-section">
  <h1>Fee Hooks</h1>

  <div class="checkbox-group">
    {#each ALL_HOOKS.slice(4, 8) as hookName}
        <label class:checked={opts.hook === hookName}>
          <input type="radio" bind:group={opts.hook} value={hookName} />
          {hookName}
          <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
            @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
            emergency response.
          </HelpTooltip>
        </label>
    {/each}
  </div>

</section>

<section class="controls-section">
  <h1>General Hooks</h1>

  <div class="checkbox-group">
    {#each ALL_HOOKS.slice(8) as hookName}
        <label class:checked={opts.hook === hookName}>
          <input type="radio" bind:group={opts.hook} value={hookName} />
          {hookName}
          <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
            @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
            emergency response.
          </HelpTooltip>
        </label>
    {/each}
  </div>

</section>

<section class="controls-section">
  <h1>Features</h1>

  <div class="checkbox-group">
    <label class:checked={opts.pausable}>
      <input type="checkbox" bind:checked={opts.pausable} />
      Pausable
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
        Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
        emergency response.
      </HelpTooltip>
    </label>
  </div>
</section>

<ExpandableToggleRadio
  label="Shares"
  bind:value={opts.shares.options}
  defaultValue="ERC20"
  helpContent="Use ERC20 or ERC6909 to represent shares of a pool."
  helpLink="https://docs.openzeppelin.com/contracts/api/token/erc20"
>
  <div class="checkbox-group">
    <label class:checked={opts.shares.options === 'ERC20'}>
      <input type="radio" bind:group={opts.shares.options} value="ERC20" />
      ERC-20
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/token/erc20">
        ERC-20 is the standard for fungible tokens.
      </HelpTooltip>
    </label>

    <label class:checked={opts.shares.options === 'ERC6909'}>
      <input type="radio" bind:group={opts.shares.options} value="ERC6909" />
      ERC-6909
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/token/erc6909">
        ERC-6909 is a standard for non-fungible tokens.
      </HelpTooltip>
    </label>
  </div>
  <div style="height: 0.5rem;"></div>
  <div class="grid grid-cols-[2fr,1fr] gap-2">
    <label class="labeled-input">
      <span>Name</span>
      <input bind:value={opts.shares.name} />
    </label>

    <label class="labeled-input">
      <span>Symbol</span>
      <input bind:value={opts.shares.symbol} />
    </label>
  </div>
</ExpandableToggleRadio>

<AccessControlSection bind:access={opts.access} required={requireAccessControl} />

<section class="controls-section">
  <h1>Utilities</h1>

  <div class="checkbox-group">
    <label class:checked={opts.currencySettler}>
      <input type="checkbox" bind:checked={opts.currencySettler} />
      CurrencySettler
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
        @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
        emergency response.
      </HelpTooltip>
    </label>

    <label class:checked={opts.safeCast}>
      <input type="checkbox" bind:checked={opts.safeCast} />
      SafeCast
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
        @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
        emergency response.
      </HelpTooltip>
    </label>

    <label class:checked={opts.transientStorage}>
      <input type="checkbox" bind:checked={opts.transientStorage} />
      Transient Storage
      <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/utils#Pausable">
        @TBD Privileged accounts will be able to pause the functionality marked as <code>whenNotPaused</code>. Useful for
        emergency response.
      </HelpTooltip>
    </label>
  </div>
</section>

@gonzaotc
Copy link
Contributor Author

I confirm that I have read and hereby agree to the OpenZeppelin Contributor License Agreement

Copy link

qodo-merge-pro bot commented Aug 13, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Missing core dependency integration

The new Uniswap Hooks package relies on @uniswap/v4-core types (e.g.,
IPoolManager) but the repo only adds @openzeppelin/uniswap-hooks; there is no
declared dependency on Uniswap v4 core or pinning of versions, risking broken
imports and incompatibilities. Add explicit, version-pinned dependencies (and
peer ranges if appropriate) for @uniswap/v4-core and any other required Uniswap
packages, and update the compatibleContractsSemver from '^0.0.0' to a meaningful
range to enforce UI compatibility checks.

Examples:

packages/core/uniswap-hooks/src/hooks.ts [132-135]
c.addImportOnly({
  name: 'IPoolManager',
  path: `@uniswap/v4-core/contracts/interfaces/IPoolManager.sol`,
});
packages/core/uniswap-hooks/package.json [23-26]
"devDependencies": {
  "@openzeppelin/uniswap-hooks": "https://github.com/OpenZeppelin/uniswap-hooks",
  "@openzeppelin/contracts": "^5.3.0",
  "@types/node": "^20.0.0",

Solution Walkthrough:

Before:

// packages/core/uniswap-hooks/package.json
"devDependencies": {
  "@openzeppelin/uniswap-hooks": "...",
  "@openzeppelin/contracts": "...",
  // Missing @uniswap/v4-core
}

// packages/core/uniswap-hooks/src/utils/version.ts
export const compatibleContractsSemver = '^0.0.0';

After:

// packages/core/uniswap-hooks/package.json
"dependencies": {
  "@uniswap/v4-core": "^1.0.0" // Or appropriate version
},
"devDependencies": {
  "@openzeppelin/uniswap-hooks": "...",
  "@openzeppelin/contracts": "...",
}

// packages/core/uniswap-hooks/src/utils/version.ts
export const compatibleContractsSemver = '^1.0.0';
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a critical packaging flaw where a required dependency @uniswap/v4-core is used but not declared, which would break the package for consumers.

High
Possible issue
Add library to imports automatically

The addUsing method should also add the library to imports to ensure it's
properly imported in the generated contract. Without this, the library may be
referenced but not imported, causing compilation errors.

packages/core/solidity/src/contract.ts [137-144]

 addUsing(library: ImportContract, usingFor: string): boolean {
   const exists = this.using.some(u => u.library.name === library.name && u.usingFor === usingFor);
   if (!exists) {
     this.using.push({ library, usingFor });
+    this.addImportOnly(library);
     return true;
   }
   return false;
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix, as using for statements require the library to be imported, and without this change, the generated code would fail to compile.

High
Fix string capitalization logic

The sanitization logic is incorrect - it only takes the first character and
uppercases it, which will always result in a single character like "H". This
should properly capitalize the entire string to match the expected "Hooks"
format.

packages/core/uniswap-hooks/src/kind.ts [5-13]

 export function sanitizeKind(kind: unknown): Kind {
   if (typeof kind === 'string') {
-    const sanitized = kind.trim().toLowerCase().charAt(0).toUpperCase();
-    if (isKind(sanitized)) {
-      return sanitized;
+    const sanitized = kind.trim().toLowerCase();
+    const capitalized = sanitized.charAt(0).toUpperCase() + sanitized.slice(1);
+    if (isKind(capitalized)) {
+      return capitalized;
     }
   }
   return 'Hooks';
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This is a critical bug fix for the sanitizeKind function, as the original logic was incorrect and would always return the default value 'Hooks' instead of the sanitized input.

High
Possible issue
Fix library name interpolation

The transformName helper returns a transformed import contract, not the string
name. Interpolating it directly will produce [object Object] in output. Extract
and use the transformed name property (or stringify appropriately) to avoid
invalid Solidity.

packages/core/solidity/src/print.ts [279]

-return entries.map(u => `using ${transformName(u.library)} for ${u.usingFor};`);
+return entries.map(u => {
+  const lib = transformName(u.library);
+  const libName = typeof lib === 'string' ? lib : lib.name;
+  return `using ${libName} for ${u.usingFor};`;
+});
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a bug where an object would be interpolated into a string, leading to invalid Solidity code using [object Object].... The fix is crucial for the new using statement printing functionality to work correctly.

High
General
Correct ERC-6909 description

ERC-6909 is a multi-token (fungible) standard, not non-fungible. Correct the
tooltip text to avoid misleading users configuring shares.

packages/ui/src/uniswap-hooks/HooksControls.svelte [118-120]

 <HelpTooltip link="https://docs.openzeppelin.com/contracts/api/token/erc6909">
-  ERC-6909 is a standard for non-fungible tokens.
+  ERC-6909 is a standard for multi-token fungible ledgers.
 </HelpTooltip>

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why: The suggestion corrects a misleading description of the ERC-6909 standard in a UI tooltip, which improves user understanding and prevents potential misconfiguration.

Low
Align default contract name

Defaulting the generated contract name to "MyToken" is inconsistent with the
Hooks flow where the default is "MyHook". Use the same default to match UI
labels and avoid confusion in Remix/code output.

packages/ui/src/uniswap-hooks/App.svelte [49]

-let contract: Contract = new ContractBuilder(initialOpts.name ?? 'MyToken');
+let contract: Contract = new ContractBuilder(initialOpts.name ?? 'MyHook');

[Suggestion processed]

Suggestion importance[1-10]: 5

__

Why: This suggestion improves UI consistency by aligning the default contract name in the code generation logic with the default name used in the UI controls, reducing potential user confusion.

Low
  • More

Copy link

socket-security bot commented Aug 13, 2025

Caution

Review the following alerts detected in dependencies.

According to your organization's Security Policy, you must resolve all "Block" alerts before proceeding. Learn more about Socket for GitHub.

Action Severity Alert  (click "▶" to expand/collapse)
Block Low
prettier@3.6.2 is a AI-detected potential code anomaly.

Notes: No definitive malware detected in this fragment. The main security concern is supply-chain risk from dynamically loading plugins from potentially untrusted sources. To mitigate, enforce strict plugin provenance, disable remote plugin loading, verify plugin integrity, and apply least-privilege execution for plugins.

Confidence: 1.00

Severity: 0.60

From: package.jsonnpm/prettier@3.6.2

ℹ Read more on: This package | This alert | What is an AI-detected potential code anomaly?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/prettier@3.6.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

@gonzaotc gonzaotc changed the title Hooks Wizard v1 [WIP] Hooks Wizard v0.0.0 [WIP] Aug 13, 2025
Copy link

socket-security bot commented Aug 27, 2025

ericglau added a commit to ericglau/contracts-wizard that referenced this pull request Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants