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

Allow Swap App restriction #1458

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Allow Swap App restriction #1458

merged 2 commits into from
Apr 26, 2024

Conversation

fmrsabino
Copy link
Collaborator

Summary

Adds the ability to restrict the decoding functionality to specific Swap applications. Swap applications can be identified with Metadata that is added to each order by the respective apps. The property which contains this metadata is fullAppDatahttps://explorer.cow.fi/appdata?tab=encode

Changes

  • Two configuration properties were added:
    • SWAPS_RESTRICT_APPS – this configuration controls if the App restriction should be enabled or disabled.
    • SWAPS_ALLOWED_APPS – this configuration contains the comma separated collection of Swap apps which are allowed to be decoded. This configuration only has an effect if SWAPS_RESTRICT_APPS is set to true.

@fmrsabino fmrsabino self-assigned this Apr 25, 2024
@fmrsabino fmrsabino requested a review from a team as a code owner April 25, 2024 10:51
@coveralls
Copy link

coveralls commented Apr 25, 2024

Pull Request Test Coverage Report for Build 8845849526

Details

  • 7 of 9 (77.78%) changed or added relevant lines in 1 file are covered.
  • 33 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.1%) to 92.271%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/routes/transactions/mappers/common/swap-order.mapper.ts 7 9 77.78%
Files with Coverage Reduction New Missed Lines %
src/routes/cache-hooks/cache-hooks.service.ts 1 98.88%
src/routes/transactions/mappers/common/swap-order.mapper.ts 1 41.67%
src/routes/transactions/entities/tests/human-description.builder.ts 4 60.0%
src/routes/transactions/entities/confirmation-view/confirmation-view.entity.ts 12 64.0%
src/routes/transactions/entities/swaps/swap-order-info.entity.ts 15 57.14%
Totals Coverage Status
Change from base Build 8831003610: -0.1%
Covered Lines: 6882
Relevant Lines: 7195

💛 - Coveralls

@@ -29,6 +36,13 @@ export class SwapOrderMapper {
orderUid,
});

if (
this.restrictApps &&
!this.allowedApps.has(order.fullAppData['appCode'])
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to add a comment with a link to the CoW docs here. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe in the Config itself what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Either is a good idea for future reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added in 37f640f

@fmrsabino fmrsabino requested a review from iamacook April 26, 2024 09:07
@fmrsabino fmrsabino merged commit f90a054 into main Apr 26, 2024
16 checks passed
@fmrsabino fmrsabino deleted the restrict-apps-decoding branch April 26, 2024 09:41
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.

None yet

3 participants