Skip to content

Fix MultiTokenPeriodBuilder to accept a config object#80

Merged
jeffsmale90 merged 3 commits into
mainfrom
fix/multi-token-period-builder
Sep 18, 2025
Merged

Fix MultiTokenPeriodBuilder to accept a config object#80
jeffsmale90 merged 3 commits into
mainfrom
fix/multi-token-period-builder

Conversation

@jeffsmale90

Copy link
Copy Markdown
Collaborator

MultiTokenPeriodBuilder accepts an array as it's config. This is problematic when applied to the declarative API, which expects to be able to squash the config object onto the declarative config.

📝 Description

Previously the builder would accept an array builder.addCaveat("multiTokenPeriod", [ { token, ... } ]).

With this change, the builder now accepts an object with tokenConfigs. builder.addCaveat("multiTokenPeriod", { tokenConfigs: [ { token, ... } ] }).

Although it doesn't necessarily make sense to use this caveat to attenuate any of the existing scopes, it's important to have consistency across all caveat types.

@jeffsmale90 jeffsmale90 requested a review from a team as a code owner September 16, 2025 05:20
Comment on lines +724 to +733
.addCaveat('multiTokenPeriod', {
tokenConfigs: [
{
token: erc20TokenAddress,
periodAmount,
periodDuration,
startDate: futureStartDate,
},
],
} as any)

@hanzel98 hanzel98 Sep 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it needed to use as any ?
the same is done below but there is one example above without it

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Definitely shouldn't have these! I've fixed it, thanks!

hanzel98
hanzel98 previously approved these changes Sep 16, 2025
@hanzel98

Copy link
Copy Markdown
Contributor

I wonder why you needed to add tsx

@jeffsmale90 jeffsmale90 merged commit 8fb0100 into main Sep 18, 2025
17 checks passed
@jeffsmale90 jeffsmale90 deleted the fix/multi-token-period-builder branch September 18, 2025 06:24
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.

2 participants