reflect review comments

.github/workflows/ci.yml

@@ -32,9 +32,9 @@ jobs:
       - name: Build e2e
         working-directory: ./test/sdk-package-test
         run: yarn run build
-        env:
-          PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
 
       - name: Run e2e tests
         working-directory: ./test/sdk-package-test
         run: yarn run test:e2e
+         env:
+          PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}

src/ajv.ts

@@ -1,6 +1,7 @@
 import Ajv, { type ErrorObject } from 'ajv';
 import { parseUnits } from 'ethers';
-import { type SplitRecipient } from './types';
+import { RewardsSplitPayload, SplitRecipient } from './types';
+import { DEFAULT_RETROACTIVE_FUNDING_REWARDS_ONLY_SPLIT } from './constants';
 
 const validDepositAmounts = (data: boolean, deposits: string[]): boolean => {
   let sum = 0;
@@ -29,13 +30,13 @@ const validDepositAmounts = (data: boolean, deposits: string[]): boolean => {
 
 const validateSplitRecipients = (
   _: boolean,
-  data: SplitRecipient[],
+  data: RewardsSplitPayload,
 ): boolean => {
-  const totalPercentage = data.reduce(
+  const splitPercentage = data.splitRecipients.reduce(
     (acc: number, curr: SplitRecipient) => acc + curr.percentAllocation,
     0,
   );
-  return totalPercentage === 99;
+  return splitPercentage + (data.ObolRAFSplit || DEFAULT_RETROACTIVE_FUNDING_REWARDS_ONLY_SPLIT) === 100;
 };
 
 export function validatePayload(

src/constants.ts

@@ -157,3 +157,5 @@ export const CHAIN_CONFIGURATION = {
     RETROACTIVE_FUNDING_ADDRESS: '0x43F641fA70e09f0326ac66b4Ef0C416EaEcBC6f5',
   },
 };
+
+export const DEFAULT_RETROACTIVE_FUNDING_REWARDS_ONLY_SPLIT= 1

src/index.ts

@@ -1,4 +1,4 @@
-import { type Provider, type Signer } from 'ethers';
+import { ZeroAddress, type Provider, type Signer } from 'ethers';
 import { v4 as uuidv4 } from 'uuid';
 import { Base } from './base.js';
 import {
@@ -15,6 +15,7 @@ import {
   TERMS_AND_CONDITIONS_HASH,
   AVAILABLE_SPLITTER_CHAINS,
   CHAIN_CONFIGURATION,
+  DEFAULT_RETROACTIVE_FUNDING_REWARDS_ONLY_SPLIT,
 } from './constants.js';
 import { ConflictError } from './errors.js';
 import {
@@ -36,7 +37,6 @@ import {
   formatSplitRecipients,
   handleDeployRewardsSplitter,
   predictSplitterAddress,
-  RETROACTIVE_FUNDING_SPLIT,
 } from './splitHelpers.js';
 import { isContractAvailable } from './utils.js';
 export * from './types.js';
@@ -121,6 +121,10 @@ export class Client extends Base {
     splitRecipients,
     principalRecipient,
     validatorsSize,
+    ObolRAFSplit = DEFAULT_RETROACTIVE_FUNDING_REWARDS_ONLY_SPLIT,
+    distributorFee = 0,
+    controllerAddress = ZeroAddress,
+
   }: RewardsSplitPayload): Promise<SplitterReturnedType> {
     // This method doesnt require T&C signature
     if (!this.signer) {
@@ -132,6 +136,9 @@ export class Client extends Base {
         splitRecipients,
         principalRecipient,
         validatorsSize,
+        ObolRAFSplit,
+        distributorFee,
+        controllerAddress
       },
       rewardsSplitterPayloadSchema,
     );
@@ -171,7 +178,7 @@ export class Client extends Base {
 
     const retroActiveFundingRecipient = {
       account: CHAIN_CONFIGURATION[this.chainId].RETROACTIVE_FUNDING_ADDRESS,
-      percentAllocation: RETROACTIVE_FUNDING_SPLIT,
+      percentAllocation: ObolRAFSplit,
     };
 
     const copiedSplitRecipients = [...splitRecipients];
@@ -185,6 +192,7 @@ export class Client extends Base {
       accounts,
       percentAllocations,
       chainId: this.chainId,
+      distributorFee
     });
     const isSplitterDeployed = await isContractAvailable(
       predictedSplitterAddress,
@@ -200,6 +208,8 @@ export class Client extends Base {
         principalRecipient,
         validatorsSize,
         chainId: this.chainId,
+        distributorFee,
+        controllerAddress
       });
 
     return { withdrawalAddress, feeRecipientAddress };

src/schema.ts

@@ -41,13 +41,11 @@ export const definitionSchema = {
         properties: {
           fee_recipient_address: {
             type: 'string',
-            minLength: 42,
-            maxLength: 42,
+            pattern: "^0x[a-fA-F0-9]{40}$"
           },
           withdrawal_address: {
             type: 'string',
-            minLength: 42,
-            maxLength: 42,
+            pattern: "^0x[a-fA-F0-9]{40}$"
           },
         },
         required: ['fee_recipient_address', 'withdrawal_address'],
@@ -75,16 +73,14 @@ export const rewardsSplitterPayloadSchema = {
         properties: {
           account: {
             type: 'string',
-            minLength: 42,
-            maxLength: 42,
+            pattern: "^0x[a-fA-F0-9]{40}$"
           },
           percentAllocation: {
             type: 'number',
           },
         },
         required: ['account', 'percentAllocation'],
       },
-      validateSplitRecipients: true,
     },
     principalRecipient: {
       type: 'string',
@@ -94,6 +90,21 @@ export const rewardsSplitterPayloadSchema = {
     validatorsSize: {
       type: 'number',
     },
+    ObolRAFSplit: {
+      type: 'number',
+      minimum: 1
+    },
+    distributorFee: {
+      type: "number",
+      maximum: 10,
+      multipleOf: 0.01
+    },
+    controllerAddress: {
+      type: 'string',
+      pattern: "^0x[a-fA-F0-9]{40}$"
+    },
+    validateSplitRecipients: true,
+
   },
-  required: ['splitRecipients', 'principalRecipient', 'validatorsSize'],
-};
+  required: ['splitRecipients', 'principalRecipient', 'validatorsSize'], 
+}

src/splitHelpers.ts

@@ -12,12 +12,8 @@ import { CHAIN_CONFIGURATION } from './constants';
 const splitMainContractInterface = new Interface(splitMainEthereumAbi);
 const owrFactoryContractInterface = new Interface(OWRFactoryContract.abi);
 
-export const RETROACTIVE_FUNDING_SPLIT = 1;
-
 // Double check if we need them as params with defaults
-const DISTRIBUTOR_FEE = 0;
 const RECOVERY_ADDRESS = '0x0000000000000000000000000000000000000000';
-const controller = '0x0000000000000000000000000000000000000000';
 
 type Call = {
   target: ETH_ADDRESS;
@@ -56,11 +52,13 @@ export const predictSplitterAddress = async ({
   accounts,
   percentAllocations,
   chainId,
+  distributorFee
 }: {
   signer: Signer;
   accounts: ETH_ADDRESS[];
   percentAllocations: number[];
   chainId: number;
+  distributorFee: number
 }): Promise<ETH_ADDRESS> => {
   const splitMainContractInstance = new Contract(
     CHAIN_CONFIGURATION[chainId].SPLITMAIN_ADDRESS,
@@ -72,7 +70,7 @@ export const predictSplitterAddress = async ({
     await splitMainContractInstance.predictImmutableSplitAddress(
       accounts,
       percentAllocations,
-      DISTRIBUTOR_FEE,
+      distributorFee,
     );
 
   return predictedSplitAddress;
@@ -87,6 +85,8 @@ export const handleDeployRewardsSplitter = async ({
   validatorsSize,
   principalRecipient,
   chainId,
+  distributorFee,
+  controllerAddress
 }: {
   signer: Signer;
   isSplitterDeployed: boolean;
@@ -96,6 +96,8 @@ export const handleDeployRewardsSplitter = async ({
   validatorsSize: number;
   principalRecipient: ETH_ADDRESS;
   chainId: number;
+  distributorFee: number;
+  controllerAddress: ETH_ADDRESS
 }): Promise<SplitterReturnedType> => {
   try {
     if (isSplitterDeployed) {
@@ -124,6 +126,8 @@ export const handleDeployRewardsSplitter = async ({
           },
           signer,
           chainId,
+          distributorFee,
+          controllerAddress
         });
 
       return {
@@ -185,15 +189,19 @@ export const deployImmutableSplitterAndOWRContracts = async ({
   splitterArgs,
   signer,
   chainId,
+  distributorFee,
+  controllerAddress
 }: {
   owrArgs: OWRArgs;
   splitterArgs: SplitArgs;
   signer: Signer;
   chainId: number;
+  distributorFee: number,
+  controllerAddress: ETH_ADDRESS
 }): Promise<{ owrAddress: ETH_ADDRESS; splitterAddress: ETH_ADDRESS }> => {
   const executeCalls: Call[] = [];
-  splitterArgs.distributorFee = DISTRIBUTOR_FEE;
-  splitterArgs.controller = controller;
+  splitterArgs.distributorFee = distributorFee;
+  splitterArgs.controller = controllerAddress;
 
   owrArgs.recoveryAddress = RECOVERY_ADDRESS;
   try {

src/types.ts

@@ -148,6 +148,15 @@ export type TotalSplitPayload = {
 
   /** Maximum number of validators with this configuration. */
   validatorsSize: number;
+
+  /** Maximum number of validators with this configuration. */
+  ObolRAFSplit?: number,
+
+  /** Fees paid to the distributor. */
+  distributorFee?: number,
+
+  /** Address that can mutate the split, should be ZeroAddress for immutable split. */
+  controllerAddress?: string,
 };
 
 /**

test/methods.test.ts

@@ -370,6 +370,21 @@ describe('createObolRewardSplit', () => {
     }
   });
 
+  test('should throw an error if ObolRAFSplit is less than 1', async () => {
+    try {
+      await clientInstance.createObolRewardSplit({
+        splitRecipients: mockSplitRecipients,
+        principalRecipient: mockPrincipalRecipient,
+        validatorsSize: mockValidatorsSize,
+        ObolRAFSplit:0.5
+      });
+    } catch (error: any) {
+      expect(error.message).toEqual(
+        'Schema compilation errors\', must be >= 1',
+      );
+    }
+  });
+
   it('should return the correct withdrawal and fee recipient addresses', async () => {
     const result = await clientInstance.createObolRewardSplit({
       splitRecipients: mockSplitRecipients,

test/sdk-package-test/utils.ts

@@ -9,7 +9,7 @@ import dotenv from 'dotenv';
 
 dotenv.config();
 
-const privateKey = process.env.PRIVATE_KEY as string; // keep it in gh secrets
+const privateKey = process.env.PRIVATE_KEY as string;
 
 const provider = new JsonRpcProvider('https://ethereum-holesky.publicnode.com');