Skip to content

Exclude types from rate limiting in approval controller#1185

Merged
matthewwalsh0 merged 3 commits into
mainfrom
feat/approval-controller-disable-rate-limiting
Apr 14, 2023
Merged

Exclude types from rate limiting in approval controller#1185
matthewwalsh0 merged 3 commits into
mainfrom
feat/approval-controller-disable-rate-limiting

Conversation

@matthewwalsh0
Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 commented Apr 14, 2023

Description

Currently all approval requests created by the ApprovalController are rate limited to a single pending approval request for each approval type and origin combination.

This is not yet required for some existing approval flows being migrated to use the ApprovalController hence the need for a way to exclude specific approval types from rate limiting to avoid any regressions in functionality and user expectation.

Changes

  • ADDED: Optional typesExcludedFromRateLimiting parameter to the ApprovalController constructor to disable rate limiting for specific approval types.
  • CHANGED: Data structure of _origins property to support multiple requests with same origin and type.

References

[Bug]: Approval request with id not found.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@matthewwalsh0 matthewwalsh0 changed the title Support excluding types from rate limiting in approval controller Exclude types from rate limiting in approval controller Apr 14, 2023
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review April 14, 2023 12:42
@matthewwalsh0 matthewwalsh0 requested a review from a team as a code owner April 14, 2023 12:42
@matthewwalsh0 matthewwalsh0 marked this pull request as draft April 14, 2023 12:51
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review April 14, 2023 14:13
Comment thread packages/approval-controller/src/ApprovalController.test.ts
Comment thread packages/approval-controller/src/ApprovalController.ts Outdated
cryptotavares
cryptotavares previously approved these changes Apr 14, 2023
Copy link
Copy Markdown
Contributor

@cryptotavares cryptotavares left a comment

Choose a reason for hiding this comment

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

Looks good! ✅
just left some very minor comments.

@matthewwalsh0 matthewwalsh0 merged commit 8354b2e into main Apr 14, 2023
@matthewwalsh0 matthewwalsh0 deleted the feat/approval-controller-disable-rate-limiting branch April 14, 2023 15:36
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