Skip to content

update ovm factory info - #153

Merged
HananINouman merged 2 commits into
mainfrom
Hanan/update-ovm
Sep 19, 2025
Merged

update ovm factory info#153
HananINouman merged 2 commits into
mainfrom
Hanan/update-ovm

Conversation

@HananINouman

@HananINouman HananINouman commented Sep 19, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • New Features

    • Expanded OVM with role-based governance, enhanced fund distribution/withdrawal controls, ownership handover, and native ETH support.
    • Richer on-chain events for improved monitoring and transparency.
    • ENS integration when deploying via the factory.
    • Added Holesky network support for the OVM Factory.
  • Changes

    • Updated OVM Factory contract addresses on Mainnet and Hoodi networks.
  • Refactor

    • Consolidated and updated OVM ABI imports to a single source for consistency.

@HananINouman
HananINouman requested a review from a team as a code owner September 19, 2025 16:20
@coderabbitai

coderabbitai Bot commented Sep 19, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Expands OVM ABIs in src/abi/OVM.ts, adding ENS parameters to the factory and a larger, role- and event-rich interface for OVM, including constructor changes and new functions/errors/events. Updates network constants and OVM factory addresses/bytecodes. Adjusts splitHelpers import to pull ABIs from ../abi/OVM.

Changes

Cohort / File(s) Summary
ABI updates
src/abi/OVM.ts
Redefines OVMFactoryContract constructor to include ENS fields; overhauls OVMContract ABI with new constructor, errors, events, roles, view/state-changing methods, and payable receive.
Network constants
src/constants.ts
Updates OVM factory addresses for Mainnet and Hoodi; adds Holesky entry with bytecode import; imports HOLESKY_OVM_FACTORY_BYTECODE.
Import path alignment
src/splits/splitHelpers.ts
Switches OVM ABI imports to ../abi/OVM; no logic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Deployer
  participant Factory as OVMFactoryContract
  participant ENS as ENS Reverse Registrar
  participant Owner as ENS Owner

  rect rgb(240,248,255)
  note over Dev,Factory: Factory deployment with ENS params
  Dev->>Factory: constructor(_ensName, _ensReverseRegistrar, _ensOwner, ...)
  Factory-->>ENS: setReverseRecord(_ensName, _ensOwner)
  ENS-->>Factory: ack
  Factory-->>Dev: deployed
  end
Loading
sequenceDiagram
  autonumber
  actor Client
  participant OVM as OVMContract
  participant DepSys as DepositSystem
  participant WithSys as WithdrawalSystem
  participant ConSys as ConsolidationSystem

  rect rgb(245,255,245)
  note over Client,OVM: Request withdrawal (payable)
  Client->>OVM: requestWithdrawal(amount, recipient, ...) + value=fee
  OVM->>WithSys: getFee/submitWithdrawal(...)
  WithSys-->>OVM: result
  OVM-->>Client: WithdrawalRequested event
  end

  rect rgb(255,248,240)
  note over Client,OVM: Distribute funds (push/pull)
  Client->>OVM: distributeFunds(...) / distributeFundsPull(...)
  OVM-->>Client: DistributeFunds event
  end

  rect rgb(240,245,255)
  note over Client,OVM: Request consolidation
  Client->>OVM: requestConsolidation(...)
  OVM->>ConSys: consolidate(...)
  ConSys-->>OVM: result
  OVM-->>Client: ConsolidationRequested event
  end

  rect rgb(250,250,255)
  note over Client,OVM: Governance and roles
  Client->>OVM: grantRoles/revokeRoles/setPrincipalRecipient/transferOwnership
  OVM-->>Client: RolesUpdated / Ownership* events
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • OisinKyne
  • agaskrobot

Poem

Hop, hop—new ABIs bloom,
ENS threads on factory loom.
Roles align, events take flight,
Withdrawals, splits, all wired right.
Bytecodes set, networks anew—
Thump-thump, I ship this for you. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "update ovm factory info" is concise and directly related to real changes in the PR (updated factory addresses and ENS-related constructor inputs), so it accurately signals a meaningful part of the changeset. The PR also contains larger OVM ABI expansions and role/event additions that the title does not fully describe, but the title is not misleading or generic. Overall the title is acceptable for brief change navigation though it could be more descriptive.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Hanan/update-ovm

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.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
65.0% Duplication on New Code (required ≤ 15%)

See analysis details on SonarQube Cloud

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (8)
src/constants.ts (2)

210-234: Place Holesky OVM_FACTORY_CONTRACT alongside other Holesky contract entries.

Pure style/readability: consider grouping OVM_FACTORY_CONTRACT with the other factory/contract entries (e.g., adjacent to OWR_FACTORY_CONTRACT) to keep consistency with the Mainnet/Hoodi blocks. No behavior change.


306-309: Optional: add Holesky public RPC URL.

If you want parity with Mainnet/Hoodi, add a Holesky entry to CHAIN_PUBLIC_RPC_URL. Not required since PROVIDER_MAP covers it via env.

src/abi/OVM.ts (2)

1-3: Treat ABIs as canonical JSON: silence quote/quote-props lint and add EOF newline.

The ABIs intentionally use JSON-style double quotes and quoted keys. Disable the noisy rules locally and add the missing newline.

Apply:

+/* eslint-disable quote-props, @typescript-eslint/quotes */
 export const OVMFactoryContract = {
   abi: [{ "inputs": [{ "internalType": "address", "name": "_consolidationSystemContract", "type": "address" }, { "internalType": "address", "name": "_withdrawalSystemContract", "type": "address" }, { "internalType": "address", "name": "_depositSystemContract", "type": "address" }, { "internalType": "string", "name": "_ensName", "type": "string" }, { "internalType": "address", "name": "_ensReverseRegistrar", "type": "address" }, { "internalType": "address", "name": "_ensOwner", "type": "address" }], "stateMutability": "nonpayable", "type": "constructor" }, { "inputs": [], "name": "Invalid_Owner", "type": "error" }, { "inputs": [], "name": "Invalid__Recipients", "type": "error" }, { "inputs": [], "name": "Invalid__ThresholdTooLarge", "type": "error" }, { "inputs": [], "name": "Invalid__ZeroThreshold", "type": "error" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "ovm", "type": "address" }, { "indexed": true, "internalType": "address", "name": "owner", "type": "address" }, { "indexed": false, "internalType": "address", "name": "principalRecipient", "type": "address" }, { "indexed": false, "internalType": "address", "name": "rewardRecipient", "type": "address" }, { "indexed": false, "internalType": "uint64", "name": "principalThreshold", "type": "uint64" }], "name": "CreateObolValidatorManager", "type": "event" }, { "inputs": [], "name": "consolidationSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "owner", "type": "address" }, { "internalType": "address", "name": "principalRecipient", "type": "address" }, { "internalType": "address", "name": "rewardRecipient", "type": "address" }, { "internalType": "uint64", "name": "principalThreshold", "type": "uint64" }], "name": "createObolValidatorManager", "outputs": [{ "internalType": "contract ObolValidatorManager", "name": "ovm", "type": "address" }], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [], "name": "depositSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "withdrawalSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }],
 };
 export const OVMContract = { abi: [{ "inputs": [{ "internalType": "address", "name": "_consolidationSystemContract", "type": "address" }, { "internalType": "address", "name": "_withdrawalSystemContract", "type": "address" }, { "internalType": "address", "name": "_depositSystemContract", "type": "address" }, { "internalType": "address", "name": "_owner", "type": "address" }, { "internalType": "address", "name": "_principalRecipient", "type": "address" }, { "internalType": "address", "name": "_rewardRecipient", "type": "address" }, { "internalType": "uint64", "name": "_principalThreshold", "type": "uint64" }], "stateMutability": "nonpayable", "type": "constructor" }, { "inputs": [], "name": "InvalidConsolidation_Failed", "type": "error" }, { "inputs": [], "name": "InvalidDistribution_TooLarge", "type": "error" }, { "inputs": [], "name": "InvalidRequest_NotEnoughFee", "type": "error" }, { "inputs": [], "name": "InvalidRequest_Params", "type": "error" }, { "inputs": [], "name": "InvalidRequest_SystemGetFee", "type": "error" }, { "inputs": [], "name": "InvalidWithdrawal_Failed", "type": "error" }, { "inputs": [], "name": "NewOwnerIsZeroAddress", "type": "error" }, { "inputs": [], "name": "NoHandoverRequest", "type": "error" }, { "inputs": [], "name": "Unauthorized", "type": "error" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "requester", "type": "address" }, { "indexed": true, "internalType": "bytes", "name": "source", "type": "bytes" }, { "indexed": true, "internalType": "bytes", "name": "target", "type": "bytes" }], "name": "ConsolidationRequested", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": false, "internalType": "uint256", "name": "principalPayout", "type": "uint256" }, { "indexed": false, "internalType": "uint256", "name": "rewardPayout", "type": "uint256" }, { "indexed": false, "internalType": "uint256", "name": "pullOrPush", "type": "uint256" }], "name": "DistributeFunds", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "newPrincipalRecipient", "type": "address" }, { "indexed": true, "internalType": "address", "name": "oldPrincipalRecipient", "type": "address" }], "name": "NewPrincipalRecipient", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "pendingOwner", "type": "address" }], "name": "OwnershipHandoverCanceled", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "pendingOwner", "type": "address" }], "name": "OwnershipHandoverRequested", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "oldOwner", "type": "address" }, { "indexed": true, "internalType": "address", "name": "newOwner", "type": "address" }], "name": "OwnershipTransferred", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "nonOVMToken", "type": "address" }, { "indexed": true, "internalType": "address", "name": "recipient", "type": "address" }, { "indexed": false, "internalType": "uint256", "name": "amount", "type": "uint256" }], "name": "RecoverNonOVMFunds", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "user", "type": "address" }, { "indexed": true, "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "RolesUpdated", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "account", "type": "address" }, { "indexed": false, "internalType": "uint256", "name": "amount", "type": "uint256" }], "name": "Withdrawal", "type": "event" }, { "anonymous": false, "inputs": [{ "indexed": true, "internalType": "address", "name": "requester", "type": "address" }, { "indexed": true, "internalType": "bytes", "name": "pubKey", "type": "bytes" }, { "indexed": false, "internalType": "uint256", "name": "amount", "type": "uint256" }], "name": "WithdrawalRequested", "type": "event" }, { "inputs": [], "name": "CONSOLIDATION_ROLE", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "RECOVER_FUNDS_ROLE", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "SET_PRINCIPAL_ROLE", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "WITHDRAWAL_ROLE", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "amountOfPrincipalStake", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "cancelOwnershipHandover", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "pendingOwner", "type": "address" }], "name": "completeOwnershipHandover", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [], "name": "consolidationSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "bytes", "name": "pubkey", "type": "bytes" }, { "internalType": "bytes", "name": "withdrawal_credentials", "type": "bytes" }, { "internalType": "bytes", "name": "signature", "type": "bytes" }, { "internalType": "bytes32", "name": "deposit_data_root", "type": "bytes32" }], "name": "deposit", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [], "name": "depositSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "distributeFunds", "outputs": [], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [], "name": "distributeFundsPull", "outputs": [], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [], "name": "fundsPendingWithdrawal", "outputs": [{ "internalType": "uint128", "name": "", "type": "uint128" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "account", "type": "address" }], "name": "getPullBalance", "outputs": [{ "internalType": "uint256", "name": "", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "user", "type": "address" }, { "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "grantRoles", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "user", "type": "address" }, { "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "hasAllRoles", "outputs": [{ "internalType": "bool", "name": "", "type": "bool" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "user", "type": "address" }, { "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "hasAnyRole", "outputs": [{ "internalType": "bool", "name": "", "type": "bool" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "owner", "outputs": [{ "internalType": "address", "name": "result", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "pendingOwner", "type": "address" }], "name": "ownershipHandoverExpiresAt", "outputs": [{ "internalType": "uint256", "name": "result", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "principalRecipient", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [], "name": "principalThreshold", "outputs": [{ "internalType": "uint64", "name": "", "type": "uint64" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "nonOVMToken", "type": "address" }, { "internalType": "address", "name": "recipient", "type": "address" }], "name": "recoverFunds", "outputs": [], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [], "name": "renounceOwnership", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "renounceRoles", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "bytes[]", "name": "sourcePubKeys", "type": "bytes[]" }, { "internalType": "bytes", "name": "targetPubKey", "type": "bytes" }], "name": "requestConsolidation", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [], "name": "requestOwnershipHandover", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "bytes[]", "name": "pubKeys", "type": "bytes[]" }, { "internalType": "uint64[]", "name": "amounts", "type": "uint64[]" }], "name": "requestWithdrawal", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "user", "type": "address" }, { "internalType": "uint256", "name": "roles", "type": "uint256" }], "name": "revokeRoles", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [], "name": "rewardRecipient", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "user", "type": "address" }], "name": "rolesOf", "outputs": [{ "internalType": "uint256", "name": "roles", "type": "uint256" }], "stateMutability": "view", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "newPrincipalRecipient", "type": "address" }], "name": "setPrincipalRecipient", "outputs": [], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "newOwner", "type": "address" }], "name": "transferOwnership", "outputs": [], "stateMutability": "payable", "type": "function" }, { "inputs": [{ "internalType": "address", "name": "account", "type": "address" }], "name": "withdraw", "outputs": [], "stateMutability": "nonpayable", "type": "function" }, { "inputs": [], "name": "withdrawalSystemContract", "outputs": [{ "internalType": "address", "name": "", "type": "address" }], "stateMutability": "view", "type": "function" }, { "stateMutability": "payable", "type": "receive" }] };
+
+// (ensure a trailing newline is present)

1-4: Freeze/validate ABI type (optional).

To prevent accidental mutation and improve type safety with ethers v6:

Apply:

- export const OVMFactoryContract = {
-  abi: [ ... ],
- };
+export const OVMFactoryContract = {
+  abi: [ /* unchanged */ ] as const,
+} as const;
 
- export const OVMContract = { abi: [ ... ] };
+export const OVMContract = { abi: [ /* unchanged */ ] as const } as const;

If you prefer stronger typing, you can instead annotate with satisfies { abi: import('ethers').InterfaceAbi }.

src/splits/splitHelpers.ts (4)

748-757: Avoid brittle event indexing — parse by signature to extract OVM address.

Assuming logs[1] is the factory event is fragile. Decode by event name via Interface.

Apply:

-    const receipt = await tx.wait();
-
-    // Extract OVM address from logs
-    const ovmAddressLog = receipt?.logs[1]?.topics[1];
-    if (!ovmAddressLog) {
-      throw new Error('Failed to extract OVM address from transaction logs');
-    }
-
-    const ovmAddress = '0x' + ovmAddressLog.slice(26, 66);
+    const receipt = await tx.wait();
+    // Extract OVM address from factory event by decoding logs
+    const iface = new Interface(OVMFactoryContract.abi);
+    const parsed = receipt.logs
+      .map(log => {
+        try {
+          return iface.parseLog(log);
+        } catch {
+          return null;
+        }
+      })
+      .find(p => p?.name === 'CreateObolValidatorManager');
+    if (!parsed) {
+      throw new Error('CreateObolValidatorManager event not found in transaction logs');
+    }
+    const ovmAddress = parsed.args.ovm as string;
     return ovmAddress;

855-861: Same here: extract OVM address from multicall3 receipt by decoding, not by count heuristics.

Counting logs to pick an index will break as events change. Parse the event.

Apply:

-    if (logsCount === 2) {
-      ovmAddress = '0x' + executeMultiCalls?.logs[1]?.topics[1]?.slice(26, 66);
-    } else if (logsCount === 5) {
-      ovmAddress = '0x' + executeMultiCalls?.logs[4]?.topics[1]?.slice(26, 66);
-    } else {
-      ovmAddress = '0x' + executeMultiCalls?.logs[7]?.topics[1]?.slice(26, 66);
-    }
+    const iface = new Interface(OVMFactoryContract.abi);
+    const parsed = executeMultiCalls.logs
+      .map((log: any) => {
+        try {
+          return iface.parseLog(log);
+        } catch {
+          return null;
+        }
+      })
+      .find((p: any) => p?.name === 'CreateObolValidatorManager');
+    ovmAddress = parsed?.args?.ovm as string | undefined;

559-571: Fix return types of encode*TxData helpers (they return calldata, not addresses).

Both helpers are typed as returning ETH_ADDRESS but return ABI-encoded calldata strings. Adjust types to string to avoid confusion and misuse.

Apply:

-const encodeCreateSplitTxData = (
+const encodeCreateSplitTxData = (
   accounts: ETH_ADDRESS[],
   percentAllocations: number[],
   distributorFee: number,
   controller: ETH_ADDRESS,
-): ETH_ADDRESS => {
+): string => {
   return splitMainContractInterface.encodeFunctionData('createSplit', [
     accounts,
     percentAllocations,
     distributorFee,
     controller,
   ]);
 };
 
-const encodeCreateOWRecipientTxData = (
+const encodeCreateOWRecipientTxData = (
   recoveryAddress: ETH_ADDRESS,
   principalRecipient: ETH_ADDRESS,
   rewardRecipient: ETH_ADDRESS,
   amountOfPrincipalStake: number,
-): ETH_ADDRESS => {
+): string => {
   return owrFactoryContractInterface.encodeFunctionData('createOWRecipient', [
     recoveryAddress,
     principalRecipient,
     rewardRecipient,
     parseEther(amountOfPrincipalStake.toString()),
   ]);
 };

Also applies to: 573-585


631-670: Return branded type for predicted SplitV2 address (optional).

predictSplitV2Address returns a checksummed address. Consider returning ETH_ADDRESS for consistency with other helpers.

-}): Promise<string> => {
+}): Promise<ETH_ADDRESS> => {
 ...
-    return predictedAddress;
+    return predictedAddress as ETH_ADDRESS;

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/abi/OVM.ts (3)

441-445: uint128 return — avoid JS number coercion

fundsPendingWithdrawal returns uint128. Ensure clients keep this as bigint/hex and never coerce to Number.


116-116: Harden ABI typing with const assertions

Use const assertions to preserve literal types and improve viem/wagmi inference.

Apply:

-export const OVMFactoryContract = {
+export const OVMFactoryContract = {
   abi: [
     /* … */
   ],
-};
+} as const;
-export const OVMContract = {
+export const OVMContract = {
   abi: [
     /* … */
   ],
-};
+} as const;

Optional (if using viem types):

import type { Abi } from 'viem';
export const OVMFactoryAbi: Abi = OVMFactoryContract.abi;
export const OVMAbi: Abi = OVMContract.abi;

Also applies to: 624-624


622-623: Receive payable present — ensure monitoring

Contract can receive ETH via receive(). Make sure monitoring/alerting accounts for direct transfers that don’t emit deposit events.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6de8f8c and e3e45e1.

📒 Files selected for processing (2)
  • src/abi/OVM.ts (2 hunks)
  • src/constants.ts (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/constants.ts
🔇 Additional comments (6)
src/abi/OVM.ts (6)

158-343: Events and indexed fields changed — update listeners/subgraphs

src/abi/OVM.ts (lines 158–343) adds/changes events and indexed params: ConsolidationRequested, DistributeFunds, NewPrincipalRecipient, OwnershipHandoverCanceled/Requested, OwnershipTransferred, RecoverNonOVMFunds, RolesUpdated, WithdrawalRequested/Withdrawal. Repo search for subscriptions/filters returned no matches. Manually verify and update event listeners, web3/ethers filters, ABI bindings, and any subgraph mappings to match the new event names and indexed parameters.


20-26: ENS args added — confirm deploys/config across networks

  • Constructor changed: src/abi/OVM.ts and src/abi/OWR.ts now require _ensName, _ensReverseRegistrar, _ensOwner — this changes constructor arg order and therefore factory bytecode/address.
  • Check src/constants.ts OVM_FACTORY_CONTRACT entries and ensure each listed address is a factory deployed with the new constructor (or update constants after redeploy).
  • No local deploy/instantiation found (no matches for "new OVMFactory" or "deploy(OVMFactory)"); locate/update your deploy scripts/CI to pass the three ENS args and re-run deployments if needed.

333-334: pubKey vs pubkey naming — verify downstream clients/indexers

Event WithdrawalRequested in src/abi/OVM.ts uses name 'pubKey' (src/abi/OVM.ts:333) while deposit/function inputs use 'pubkey' (src/abi/OVM.ts:404). BatchDeposit ABI also uses 'pubKey' (src/abi/BatchDeposit.ts:49). Internal tests/types mostly use lowercase 'pubkey' and some helpers map to 'pubKey' (src/eoa/eoaHelpers.ts:91). Confirm external indexers/ETL and client consumers handle the discrepancy or standardize ABI/clients.


382-394: Double‑check payable governance/role functions — ensure UIs don’t send ETH

ABI functions marked payable: cancelOwnershipHandover, completeOwnershipHandover, grantRoles, revokeRoles, renounceRoles, requestOwnershipHandover, transferOwnership. Frontends/backends must not attach non‑zero value in call overrides (overrides.value or similar) unless protocol fees are required.

Verification: the provided rg scan returned no output (no call sites found or search failed) — this is inconclusive; manual verification required. Locations: src/abi/OVM.ts lines 382–394, 460–462, 524–536, 548–553, 569–573, 605–607.

Suggested actions: search UI/backends for calls to these functions and for 'overrides'/'value', add tests asserting value==0 for governance calls, or make functions non‑payable if no value is expected.


447-452: getPullBalance now requires address param — no internal zero‑arg callsites

Search found only ABI definitions declaring getPullBalance(address) in src/abi/OVM.ts and src/abi/OWR.ts; no zero‑arg calls in the repo. This is a breaking API change for external consumers — add release/migration notes or provide a backward‑compatible wrapper.


122-147: OVM constructor widened — ensure Factory.create passes correct args/order

createObolValidatorManager call sites only supply (owner, principalRecipient, rewardRecipient, principalThreshold) (see src/splits/splitHelpers.ts:741 and :883) while the OVM constructor requires (consolidationSystemContract, withdrawalSystemContract, depositSystemContract, owner, principalRecipient, rewardRecipient, principalThreshold) (src/abi/OVM.ts). Confirm the factory internally provides the three system-contract addresses in that exact order when deploying the OVM to avoid silent mis-wiring.

Comment thread src/abi/OVM.ts
Comment on lines 118 to +146
abi: [
{
type: 'constructor',
inputs: [
{
internalType: 'address',
name: '_consolidationSystemContract',
type: 'address',
internalType: 'address',
},
{
internalType: 'address',
name: '_withdrawalSystemContract',
type: 'address',
internalType: 'address',
},
{
internalType: 'address',
name: '_depositSystemContract',
type: 'address',
internalType: 'address',
},
{ name: '_owner', type: 'address', internalType: 'address' },
{ internalType: 'address', name: '_owner', type: 'address' },
{
internalType: 'address',
name: '_principalRecipient',
type: 'address',
internalType: 'address',
},
{ name: '_rewardRecipient', type: 'address', internalType: 'address' },
{ name: '_principalThreshold', type: 'uint64', internalType: 'uint64' },
{ internalType: 'address', name: '_rewardRecipient', type: 'address' },
{ internalType: 'uint64', name: '_principalThreshold', type: 'uint64' },
],
stateMutability: 'nonpayable',
type: 'constructor',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Semver: breaking ABI surface — bump major and note in changelog

Constructor and several function signatures/returns changed; treat this as a major version bump for SDK consumers and document migration steps.

Also applies to: 345-371, 426-438


🏁 Script executed:

#!/bin/bash
# Verify package version bump and changelog entry
fd -a 'package.json' | xargs -I{} jq -r '.version' {}
rg -n 'CHANGELOG|changelog' -C2

Length of output: 10906


Major semver bump required — ABI-breaking changes must be documented

  • package.json(s) still at 2.11.3 and 0.1.0 (no major bump detected).
  • CHANGELOG.md shows older entries (latest 18 March 2024) and contains no migration notes for these ABI changes.

Affected: src/abi/OVM.ts lines 118–146, 345–371, 426–438.

Bump the major version (e.g., 3.0.0) in all package.json files and add a changelog entry with clear migration steps for SDK consumers before merging.

🤖 Prompt for AI Agents
In src/abi/OVM.ts around lines 118–146 (and also update references at 345–371
and 426–438), the ABI changes are breaking and require a major version bump and
migration notes; update all package.json files to a new major version (e.g.,
3.0.0) consistently, add a new CHANGELOG.md entry describing the ABI changes and
clear migration steps for SDK consumers (what changed in constructor and
affected methods, any parameter renames/types, and example code to migrate), and
ensure the changelog date and version header match the new major bump before
merging.

@HananINouman
HananINouman merged commit 7e11b43 into main Sep 19, 2025
4 of 6 checks passed
@HananINouman
HananINouman deleted the Hanan/update-ovm branch September 19, 2025 16:40
@coderabbitai coderabbitai Bot mentioned this pull request Dec 4, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Mar 14, 2026
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