Merge pull request #445 from dappnode/fix/validator-settings-updates

Restore missing validator keys and simplify settings updates

Author: 3alpha

packages/brain/src/modules/apiServers/launchpad/routes/feeRecipients/route.ts

@@ -41,12 +41,14 @@ import { ValidatorApi } from "../../../../apiClients/validator/index.js";
 export const createFeeRecipientsRouter = ({
   brainDb,
   signerApi,
+  signerUrl,
   validatorApi,
   beaconchainApi,
   reloadValidatorsCronTask
 }: {
   brainDb: BrainDataBase;
   signerApi: Web3SignerApi;
+  signerUrl: string;
   validatorApi: ValidatorApi;
   beaconchainApi: BeaconchainApi;
   reloadValidatorsCronTask: CronJob;
@@ -168,6 +170,7 @@ export const createFeeRecipientsRouter = ({
         brainDb,
         validatorApi,
         customValidatorUpdateRequest: validatorsToUpdate.map(({ pubkey, feeRecipient }) => ({ pubkey, feeRecipient })),
+        signerUrl,
         requestFrom: "api"
       });
 

packages/brain/src/modules/apiServers/launchpad/startLaunchpadApi.ts

@@ -40,6 +40,7 @@ export function startLaunchpadApi({
     createFeeRecipientsRouter({
       brainDb,
       signerApi,
+      signerUrl,
       validatorApi,
       beaconchainApi,
       reloadValidatorsCronTask

packages/brain/src/modules/apiServers/ui/calls/importValidators.ts

@@ -169,9 +169,7 @@ export async function importValidators({
 
       brainDb.deleteValidators(rejectedNewKeys);
       if (confirmedKeys.length > 0)
-        brainDb.updateValidators({
-          validators: Object.fromEntries(confirmedKeys.map((pubkey) => [pubkey, { importPending: undefined }]))
-        });
+        brainDb.updateValidators(confirmedKeys.map((pubkey) => ({ pubkey, importPending: undefined })));
 
       if (validatorsToPost.length === 0) return results;
 

packages/brain/src/modules/apiServers/ui/calls/index.ts

@@ -77,6 +77,7 @@ export const createRpcMethods = ({
         brainDb,
         validatorApi,
         customValidatorUpdateRequest,
+        signerUrl,
         requestFrom
       }),
     getValidators: async () =>

packages/brain/src/modules/apiServers/ui/calls/updateValidators.ts

@@ -1,73 +1,129 @@
-import { withValidatorOperationLock } from "../../../db/withValidatorOperationLock.js";
 import { prefix0xPubkey, isFeeRecipientEditable, ActionRequestOrigin } from "@stakingbrain/common";
-import logger from "../../../../modules/logger/index.js";
+import { withValidatorOperationLock } from "../../../db/withValidatorOperationLock.js";
+import logger from "../../../logger/index.js";
 import { CustomValidatorUpdateRequest } from "./types.js";
-import { StakingBrainDbUpdate } from "../../../../modules/db/types.js";
+import { StakingBrainDb } from "../../../db/types.js";
 import { CronJob } from "../../../cron/cron.js";
 import { BrainDataBase } from "../../../db/index.js";
 import { ValidatorApi } from "../../../apiClients/validator/index.js";
 import { applyValidatorBuilderConfig } from "../../../cron/reloadValidators/postValidatorsBuilderConfigsFromDb.js";
 
+type UpdateValidatorsOptions = {
+  reloadValidatorsCronTask: CronJob;
+  brainDb: BrainDataBase;
+  validatorApi: ValidatorApi;
+  signerUrl: string;
+  customValidatorUpdateRequest: CustomValidatorUpdateRequest[];
+  requestFrom?: ActionRequestOrigin;
+};
+
 /** Persist desired settings first. Periodic reload retries client failures. */
 export async function updateValidators({
   reloadValidatorsCronTask,
   brainDb,
   validatorApi,
-  customValidatorUpdateRequest,
+  signerUrl,
+  customValidatorUpdateRequest: requests,
   requestFrom
-}: {
-  reloadValidatorsCronTask: CronJob;
-  brainDb: BrainDataBase;
-  validatorApi: ValidatorApi;
-  customValidatorUpdateRequest: CustomValidatorUpdateRequest[];
-  requestFrom?: ActionRequestOrigin;
-}): Promise<void> {
+}: UpdateValidatorsOptions): Promise<void> {
   return withValidatorOperationLock(brainDb, async () => {
     reloadValidatorsCronTask.stop();
     try {
-      if (!Array.isArray(customValidatorUpdateRequest) || customValidatorUpdateRequest.length === 0)
-        throw new Error("Select at least one validator to update");
-      const dbData = brainDb.getData();
-      const updates: StakingBrainDbUpdate = {};
-      for (const validator of customValidatorUpdateRequest) {
-        if (!validator || typeof validator.pubkey !== "string") throw new Error("Validator public key is required");
-        const pubkey = prefix0xPubkey(validator.pubkey);
-        if (!dbData[pubkey]) throw new Error(`Validator ${pubkey} is not in the database`);
-        if (updates[pubkey]) throw new Error(`Duplicate validator ${pubkey}`);
-        if (Object.keys(validator).some((field) => !["pubkey", "feeRecipient", "builderConfig"].includes(field)))
-          throw new Error("Unsupported validator setting");
-        const update: StakingBrainDbUpdate[string] = {};
-        if ("feeRecipient" in validator && isFeeRecipientEditable(dbData[pubkey].tag, requestFrom))
-          update.feeRecipient = validator.feeRecipient;
-        if ("builderConfig" in validator) update.builderConfig = validator.builderConfig;
-        if (Object.keys(update).length > 0) updates[pubkey] = update;
-      }
-      if (Object.keys(updates).length === 0) throw new Error("The fee recipient can't be updated for these validators");
+      const currentValidators = brainDb.getData();
+      const updates = prepareUpdates(requests, currentValidators, requestFrom);
 
-      // Validate the entire batch before changing any persisted settings.
-      brainDb.updateValidators({ validators: updates });
-      const failures: string[] = [];
-      for (const [pubkey, update] of Object.entries(updates)) {
-        if (update.feeRecipient !== undefined)
-          await validatorApi.setFeeRecipient(update.feeRecipient, pubkey).catch((err) => {
-            logger.error("Error setting validator feeRecipient", err);
-            failures.push(`Fee recipient application failed for ${pubkey}`);
-          });
-        if ("builderConfig" in update) {
-          try {
-            await applyValidatorBuilderConfig(validatorApi, pubkey, update.builderConfig);
-          } catch (error) {
-            logger.error(error);
-            failures.push(error.message);
-          }
-        }
-      }
-      if (failures.length > 0)
-        throw new Error(
-          `Settings saved; client configuration application failed and will be retried on reload. ${failures.join("; ")}`
-        );
+      // The database validates the entire batch before persisting any settings.
+      brainDb.updateValidators(updates);
+      const importedKeys = await importMissingBuilderKeys(validatorApi, signerUrl, updates);
+      await applyUpdates(validatorApi, updates, currentValidators, importedKeys);
     } finally {
       reloadValidatorsCronTask.start();
     }
   });
 }
+
+function prepareUpdates(
+  requests: CustomValidatorUpdateRequest[],
+  currentValidators: StakingBrainDb,
+  requestFrom?: ActionRequestOrigin
+): CustomValidatorUpdateRequest[] {
+  if (!Array.isArray(requests) || requests.length === 0) throw new Error("Select at least one validator to update");
+
+  const updates: CustomValidatorUpdateRequest[] = [];
+  const seen = new Set<string>();
+  for (const request of requests) {
+    if (!request || typeof request.pubkey !== "string") throw new Error("Validator public key is required");
+    const pubkey = prefix0xPubkey(request.pubkey);
+    const current = currentValidators[pubkey];
+    if (!current) throw new Error(`Validator ${pubkey} is not in the database`);
+    if (seen.has(pubkey)) throw new Error(`Duplicate validator ${pubkey}`);
+    if (Object.keys(request).some((field) => !["pubkey", "feeRecipient", "builderConfig"].includes(field)))
+      throw new Error("Unsupported validator setting");
+
+    const { feeRecipient, builderConfig } = request;
+    const update: CustomValidatorUpdateRequest = { pubkey };
+    if (feeRecipient !== undefined && isFeeRecipientEditable(current.tag, requestFrom))
+      update.feeRecipient = feeRecipient;
+    if (builderConfig !== undefined) update.builderConfig = builderConfig;
+    if (update.feeRecipient !== undefined || update.builderConfig !== undefined) {
+      updates.push(update);
+      seen.add(pubkey);
+    }
+  }
+
+  if (updates.length === 0) throw new Error("The fee recipient can't be updated for these validators");
+  return updates;
+}
+
+/** A restart may have cleared keys before the next cron reload. */
+async function importMissingBuilderKeys(
+  validatorApi: ValidatorApi,
+  signerUrl: string,
+  updates: readonly CustomValidatorUpdateRequest[]
+): Promise<Set<string>> {
+  const builderPubkeys = updates.filter(({ builderConfig }) => builderConfig !== undefined).map(({ pubkey }) => pubkey);
+  if (builderPubkeys.length === 0) return new Set();
+
+  const remoteKeys = await validatorApi.getRemoteKeys();
+  const loaded = new Set(remoteKeys.data.map(({ pubkey }) => pubkey.toLowerCase()));
+  const missing = builderPubkeys.filter((pubkey) => !loaded.has(pubkey.toLowerCase()));
+  if (missing.length > 0)
+    await validatorApi.postRemoteKeys({ remote_keys: missing.map((pubkey) => ({ pubkey, url: signerUrl })) });
+  return new Set(missing);
+}
+
+async function applyUpdates(
+  validatorApi: ValidatorApi,
+  updates: readonly CustomValidatorUpdateRequest[],
+  currentValidators: StakingBrainDb,
+  importedKeys: Set<string>
+): Promise<void> {
+  const failures: string[] = [];
+  for (const update of updates) {
+    const { pubkey } = update;
+    let feeRecipient = update.feeRecipient;
+    if (feeRecipient === undefined && importedKeys.has(pubkey)) feeRecipient = currentValidators[pubkey].feeRecipient;
+
+    if (feeRecipient !== undefined) {
+      try {
+        await validatorApi.setFeeRecipient(feeRecipient, pubkey);
+      } catch (error) {
+        logger.error("Error setting validator feeRecipient", error);
+        failures.push(`Fee recipient application failed for ${pubkey}`);
+      }
+    }
+    if (update.builderConfig !== undefined) {
+      try {
+        await applyValidatorBuilderConfig(validatorApi, pubkey, update.builderConfig);
+      } catch (error) {
+        logger.error(error);
+        failures.push(error.message);
+      }
+    }
+  }
+
+  if (failures.length > 0)
+    throw new Error(
+      `Settings saved; client configuration application failed and will be retried on reload. ${failures.join("; ")}`
+    );
+}

packages/brain/src/modules/config/importBuilderIds.ts

@@ -35,14 +35,12 @@ export function importBuilderIds({
   };
 
   const validators = Object.entries(brainDb.getData());
-  const updates = Object.fromEntries(
-    validators
-      .filter(([, details]) => details.builderConfig === undefined)
-      .map(([pubkey]) => [pubkey, { builderConfig }])
-  );
-  const imported = Object.keys(updates).length;
+  const updates = validators
+    .filter(([, details]) => details.builderConfig === undefined)
+    .map(([pubkey]) => ({ pubkey, builderConfig }));
+  const imported = updates.length;
   const skipped = validators.length - imported;
-  if (imported > 0) brainDb.updateValidators({ validators: updates });
+  if (imported > 0) brainDb.updateValidators(updates);
 
   // Mark completion only after saving. Retries preserve existing settings and edits.
   fs.writeFileSync(markerPath, "", { mode: 0o600, flag: "wx" });

packages/brain/src/modules/cron/reloadValidators/index.ts

@@ -49,9 +49,7 @@ export async function reloadValidators(
         (pubkey) => data[pubkey].importPending && signerKeys.has(pubkey.toLowerCase())
       );
       if (confirmedImports.length > 0)
-        brainDb.updateValidators({
-          validators: Object.fromEntries(confirmedImports.map((pubkey) => [pubkey, { importPending: undefined }]))
-        });
+        brainDb.updateValidators(confirmedImports.map((pubkey) => ({ pubkey, importPending: undefined })));
       // Preserve pending records, but do not activate keys the signer has not confirmed.
       const activeData = Object.fromEntries(
         Object.entries(brainDb.getData()).filter(([pubkey]) => signerKeys.has(pubkey.toLowerCase()))

packages/brain/src/modules/cron/trackValidatorsPerformance/setActiveValidatorsLoadedInBrain.ts

@@ -78,7 +78,7 @@ async function getValidatorIndexesAndSaveInDb({
       const { pubkey } = validatorData.validator;
       const index = validatorData.index;
       validatorIndexes.push(index);
-      brainDb.updateValidators({ validators: { [pubkey]: { index: parseInt(index) } } });
+      brainDb.updateValidators([{ pubkey, index: parseInt(index) }]);
     }
   }
 

packages/brain/src/modules/db/index.ts

@@ -1,4 +1,4 @@
-import { StakingBrainDb, StakingBrainDbUpdate } from "./types.js";
+import { StakingBrainDb, ValidatorUpdate } from "./types.js";
 import { LowSync } from "lowdb";
 import { JSONFileSync } from "lowdb/node";
 import fs from "fs";
@@ -115,26 +115,25 @@ export class BrainDataBase extends LowSync<StakingBrainDb> {
   /**
    * Update supplied validator settings without erasing omitted settings.
    */
-  public updateValidators({ validators }: { validators: StakingBrainDbUpdate }): void {
+  public updateValidators(validators: readonly ValidatorUpdate[]): void {
     try {
       this.validateDb();
       this.validateUpdateValidators(validators);
       const updatedData = { ...this.data };
-      if (this.data)
-        for (const pubkey of Object.keys(validators)) {
-          if (!this.data[pubkey]) {
-            // Remove pubkeys that don't exist
-            logger.warn(`Pubkey ${pubkey} not found in the database`);
-            delete validators[pubkey];
-          } else {
-            updatedData[pubkey] = { ...this.data[pubkey], ...validators[pubkey] };
-          }
+      for (const { pubkey, ...changes } of validators) {
+        if (!this.data[pubkey]) {
+          logger.warn(`Pubkey ${pubkey} not found in the database`);
+          continue;
         }
+        updatedData[pubkey] = { ...this.data[pubkey], ...changes };
+      }
       this.ensureDbMaxSize(updatedData, true);
       this.data = updatedData;
       this.write();
     } catch (e) {
-      throw new BrainDbError(`Unable to update pubkeys ${Object.keys(validators).join(", ")}. ${e.message}`);
+      throw new BrainDbError(
+        `Unable to update pubkeys ${validators.map(({ pubkey }) => pubkey).join(", ")}. ${e.message}`
+      );
     }
   }
 
@@ -285,32 +284,31 @@ export class BrainDataBase extends LowSync<StakingBrainDb> {
     this.read();
   }
 
-  private validateUpdateValidators(validators: StakingBrainDbUpdate): void {
+  private validateUpdateValidators(validators: readonly ValidatorUpdate[]): void {
     const errors: string[] = [];
-    Object.keys(validators).forEach((pubkey) => {
-      const pubkeyDetails = validators[pubkey];
+    const seen = new Set<string>();
+    for (const validator of validators) {
+      const { pubkey } = validator;
 
       // create substring of pubkey to be used in error message
       const pubkeySubstr = shortenPubkey(pubkey);
 
       // Validate Ethereum address
       if (!isValidBlsPubkey(pubkey)) errors.push(`\n  pubkey ${pubkeySubstr}: bls is invalid`);
 
-      if (!pubkeyDetails) {
-        errors.push(`\n  pubkey ${pubkeySubstr}: pubkey details are missing`);
-        return;
-      }
+      if (seen.has(pubkey)) errors.push(`\n  pubkey ${pubkeySubstr}: duplicate update`);
+      seen.add(pubkey);
 
       // FeeRecipient
-      if ("feeRecipient" in pubkeyDetails) {
-        if (typeof pubkeyDetails.feeRecipient !== "string")
+      if ("feeRecipient" in validator) {
+        if (typeof validator.feeRecipient !== "string")
           errors.push(`\n  pubkey ${pubkeySubstr}: feeRecipient address is invalid, must be in string format`);
-        if (typeof pubkeyDetails.feeRecipient !== "string" || !isValidEcdsaPubkey(pubkeyDetails.feeRecipient))
+        if (typeof validator.feeRecipient !== "string" || !isValidEcdsaPubkey(validator.feeRecipient))
           errors.push(`\n  pubkey ${pubkeySubstr}: fee recipient is invalid`);
       }
-      if ("builderConfig" in pubkeyDetails && pubkeyDetails.builderConfig !== null)
-        validateBuilderConfig(pubkeyDetails.builderConfig);
-    });
+      if ("builderConfig" in validator && validator.builderConfig !== null)
+        validateBuilderConfig(validator.builderConfig);
+    }
     if (errors.length > 0) throw new BrainDbError(errors.join("\n"));
   }
 

packages/brain/src/modules/db/types.ts

@@ -12,8 +12,8 @@ export interface StakingBrainDb {
   [pubkey: string]: PubkeyDetails;
 }
 
-export interface StakingBrainDbUpdate {
-  [pubkey: string]: Partial<Omit<PubkeyDetails, "automaticImport" | "tag">>;
+export interface ValidatorUpdate extends Partial<Omit<PubkeyDetails, "automaticImport" | "tag">> {
+  pubkey: string;
 }
 
 export interface CustomValidators extends PubkeyDetails {