Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(relayer): Remove redundant array manipulation #1341

Closed
wants to merge 3 commits into from
Closed

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented Mar 21, 2024

This code was recently touched; it's an error without any apparent side-effects.

@@ -258,7 +258,9 @@ export class Relayer {
// Fetch unfilled deposits and filter out deposits upfront before we compute the minimum deposit confirmation
// per chain, which is based on the deposit volume we could fill.
const unfilledDeposits = await this._getUnfilledDeposits();
const allUnfilledDeposits = Object.values(unfilledDeposits.map(({ deposit }) => deposit));
const allUnfilledDeposits = Object.values(unfilledDeposits)
.flat()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the destructuring of { deposit } left deposit === undefined? Good find

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't as bad as I first thought - unfilledDeposits is already a flat array, so the extra Object.values() was just redundant code.

@pxrl pxrl changed the title fix(relayer): Subtle Object -> array mapping issue fix(relayer): Remove redundant array manipulation Mar 21, 2024
@pxrl
Copy link
Contributor Author

pxrl commented Mar 21, 2024

Closing this because it's only a superficial issue and will be cleaned up in a subsequent commit that already touches this code.

@pxrl pxrl closed this Mar 21, 2024
@pxrl pxrl deleted the pxrl/fixRelayer branch March 21, 2024 22:38
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.

3 participants