Skip to content

Commit

Permalink
fix: update objMerge implementation (#4678)
Browse files Browse the repository at this point in the history
### 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
hyperlane-xyz/hyperlane-registry#276

### Backward compatibility

should be, yes

### Testing

ci, manual testing when generating agent config files
  • Loading branch information
paulbalaji authored Oct 15, 2024
1 parent 7d7bcc1 commit f1712de
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-clocks-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperlane-xyz/utils': patch
---

Fix objMerge implementation
3 changes: 2 additions & 1 deletion typescript/cli/src/config/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ async function getStartBlocks(
chainAddresses: ChainMap<ChainAddresses>,
core: HyperlaneCore,
chainMetadata: any,
) {
): Promise<ChainMap<number | undefined>> {
return promiseObjAll(
objMap(chainAddresses, async (chain, _) => {
const indexFrom = chainMetadata[chain].index?.from;
Expand All @@ -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;
}
}),
);
Expand Down
2 changes: 1 addition & 1 deletion typescript/infra/scripts/agents/update-agent-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export async function writeAgentConfig(
'Error:',
err,
);
return 0;
return undefined;
}
},
),
Expand Down
10 changes: 6 additions & 4 deletions typescript/sdk/src/metadata/agentConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ export function buildAgentConfig(
chains: ChainName[],
multiProvider: MultiProvider,
addresses: ChainMap<HyperlaneDeploymentArtifacts>,
startBlocks: ChainMap<number>,
startBlocks: ChainMap<number | undefined>,
additionalConfig?: ChainMap<any>,
): AgentConfig {
const chainConfigs: ChainMap<AgentChainMetadata> = {};
Expand All @@ -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;
}
Expand Down
7 changes: 7 additions & 0 deletions typescript/utils/src/objects.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } };
Expand Down
42 changes: 25 additions & 17 deletions typescript/utils/src/objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,11 @@ export function pick<K extends string, V = any>(obj: Record<K, V>, 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
Expand All @@ -112,29 +115,34 @@ export function objMerge<T = any>(
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<string, any> = {};
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<string, any> = { ...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;
}

Expand Down

0 comments on commit f1712de

Please sign in to comment.