From f1712deb7f5acbbcfb669cbc205d29a2efcda932 Mon Sep 17 00:00:00 2001 From: Paul Balaji <10051819+paulbalaji@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:16:15 +0100 Subject: [PATCH] fix: update objMerge implementation (#4678) ### Description Updating the `objMerge` implementation A bug in the original implementation meant that the `update-agent-config` script did _not_ overwrite the `blocks.reorgPeriod` if there was a change, this new version does. ### Drive-by changes - gracefully handle missing startBlock data when generating agent config - fix objMerge calls that had the wrong order ### Related issues definitely want to fix this bug before attempting to update our agent configs with changes in https://github.com/hyperlane-xyz/hyperlane-registry/pull/276 ### Backward compatibility should be, yes ### Testing ci, manual testing when generating agent config files --- .changeset/dirty-clocks-repeat.md | 5 +++ typescript/cli/src/config/agent.ts | 3 +- .../scripts/agents/update-agent-config.ts | 2 +- typescript/sdk/src/metadata/agentConfig.ts | 10 +++-- typescript/utils/src/objects.test.ts | 7 ++++ typescript/utils/src/objects.ts | 42 +++++++++++-------- 6 files changed, 46 insertions(+), 23 deletions(-) create mode 100644 .changeset/dirty-clocks-repeat.md diff --git a/.changeset/dirty-clocks-repeat.md b/.changeset/dirty-clocks-repeat.md new file mode 100644 index 000000000..d9159825a --- /dev/null +++ b/.changeset/dirty-clocks-repeat.md @@ -0,0 +1,5 @@ +--- +'@hyperlane-xyz/utils': patch +--- + +Fix objMerge implementation diff --git a/typescript/cli/src/config/agent.ts b/typescript/cli/src/config/agent.ts index dff8f2255..05fa16559 100644 --- a/typescript/cli/src/config/agent.ts +++ b/typescript/cli/src/config/agent.ts @@ -87,7 +87,7 @@ async function getStartBlocks( chainAddresses: ChainMap, core: HyperlaneCore, chainMetadata: any, -) { +): Promise> { return promiseObjAll( objMap(chainAddresses, async (chain, _) => { const indexFrom = chainMetadata[chain].index?.from; @@ -103,6 +103,7 @@ async function getStartBlocks( errorRed( `❌ Failed to get deployed block to set an index for ${chain}, this is potentially an issue with rpc provider or a misconfiguration`, ); + return undefined; } }), ); diff --git a/typescript/infra/scripts/agents/update-agent-config.ts b/typescript/infra/scripts/agents/update-agent-config.ts index 4584348cf..30d64a9d9 100644 --- a/typescript/infra/scripts/agents/update-agent-config.ts +++ b/typescript/infra/scripts/agents/update-agent-config.ts @@ -101,7 +101,7 @@ export async function writeAgentConfig( 'Error:', err, ); - return 0; + return undefined; } }, ), diff --git a/typescript/sdk/src/metadata/agentConfig.ts b/typescript/sdk/src/metadata/agentConfig.ts index f8c162f86..cb570e29e 100644 --- a/typescript/sdk/src/metadata/agentConfig.ts +++ b/typescript/sdk/src/metadata/agentConfig.ts @@ -418,7 +418,7 @@ export function buildAgentConfig( chains: ChainName[], multiProvider: MultiProvider, addresses: ChainMap, - startBlocks: ChainMap, + startBlocks: ChainMap, additionalConfig?: ChainMap, ): AgentConfig { const chainConfigs: ChainMap = {}; @@ -438,9 +438,11 @@ export function buildAgentConfig( ...metadata, ...addresses[chain], ...(additionalConfig ? additionalConfig[chain] : {}), - index: { - from: startBlocks[chain], - }, + ...(startBlocks[chain] !== undefined && { + index: { + from: startBlocks[chain], + }, + }), }; chainConfigs[chain] = chainConfig; } diff --git a/typescript/utils/src/objects.test.ts b/typescript/utils/src/objects.test.ts index e454a2b17..b6fd1d012 100644 --- a/typescript/utils/src/objects.test.ts +++ b/typescript/utils/src/objects.test.ts @@ -35,6 +35,13 @@ describe('Object utilities', () => { expect(merged).to.eql({ a: 2, b: { c: ['arr2'] } }); }); + it('objMerge overwrites nested values', () => { + const obj1 = { a: { b: 10 }, c: 'value' }; + const obj2 = { a: { b: 20 } }; + const merged = objMerge(obj1, obj2); + expect(merged).to.eql({ a: { b: 20 }, c: 'value' }); + }); + it('objOmit', () => { const obj1 = { a: 1, b: { c: ['arr1'], d: 'string' } }; const obj2 = { a: true, b: { c: true } }; diff --git a/typescript/utils/src/objects.ts b/typescript/utils/src/objects.ts index b97d4e662..680c5d579 100644 --- a/typescript/utils/src/objects.ts +++ b/typescript/utils/src/objects.ts @@ -99,8 +99,11 @@ export function pick(obj: Record, keys: K[]) { } /** - * Returns a new object that recursively merges b into a - * Where there are conflicts, b takes priority over a + * Returns a new object that recursively merges B into A + * Where there are conflicts, B takes priority over A + * If B has a value for a key that A does not have, B's value is used + * If B has a value for a key that A has, and both are objects, the merge recurses into those objects + * If B has a value for a key that A has, and both are arrays, the merge concatenates them with B's values taking priority * @param a - The first object * @param b - The second object * @param max_depth - The maximum depth to recurse @@ -112,29 +115,34 @@ export function objMerge( max_depth = 10, mergeArrays = false, ): T { + // If we've reached the max depth, throw an error if (max_depth === 0) { throw new Error('objMerge tried to go too deep'); } + // If either A or B is not an object, return the other value if (!isObject(a) || !isObject(b)) { - return (b ? b : a) as T; + return (b ?? a) as T; } - const ret: Record = {}; - const aKeys = new Set(Object.keys(a)); - const bKeys = new Set(Object.keys(b)); - const allKeys = new Set([...aKeys, ...bKeys]); - for (const key of allKeys.values()) { - if (aKeys.has(key) && bKeys.has(key)) { - if (mergeArrays && Array.isArray(a[key]) && Array.isArray(b[key])) { - ret[key] = [...b[key], ...a[key]]; - } else { - ret[key] = objMerge(a[key], b[key], max_depth - 1, mergeArrays); - } - } else if (aKeys.has(key)) { - ret[key] = a[key]; - } else { + // Initialize returned object with values from A + const ret: Record = { ...a }; + // Iterate over keys in B + for (const key in b) { + // If both A and B have the same key, recursively merge the values from B into A + if (isObject(a[key]) && isObject(b[key])) { + ret[key] = objMerge(a[key], b[key], max_depth - 1, mergeArrays); + } + // If A & B are both arrays, and we're merging them, concatenate them with B's values taking priority before A + else if (mergeArrays && Array.isArray(a[key]) && Array.isArray(b[key])) { + ret[key] = [...b[key], ...a[key]]; + } + // If B has a value for the key, set the value to B's value + // This better handles the case where A has a value for the key, but B does not + // In which case we want to keep A's value + else if (b[key] !== undefined) { ret[key] = b[key]; } } + // Return the merged object return ret as T; }