Skip to content

Prevent ApprovalController counting mismatch#356

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-approval-store-counting-bug
Feb 18, 2021
Merged

Prevent ApprovalController counting mismatch#356
Gudahtt merged 1 commit intodevelopfrom
fix-approval-store-counting-bug

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Feb 18, 2021

The ApprovalController was not great at counting. If you tried to delete an approval that didn't exist, or if you tried to add an approval that already existed, it would miscount the current number of pending approvals.

Validation was in place already to prevent both scenarios, so in practice this shouldn't be possible without calling private functions. But still, preventing it entirely was trivial, so here it is.

The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
@Gudahtt Gudahtt requested a review from a team as a code owner February 18, 2021 16:06
@Gudahtt Gudahtt requested a review from rekmarks February 18, 2021 16:07
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

Good catch!

@Gudahtt Gudahtt merged commit af700d8 into develop Feb 18, 2021
@Gudahtt Gudahtt deleted the fix-approval-store-counting-bug branch February 18, 2021 19:06
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The `ApprovalController` was not great at counting. If you tried to
delete an approval that didn't exist, or if you tried to add an
approval that already existed, it would miscount the current number
of pending approvals.

Validation was in place already to prevent both scenarios, so in
practice this shouldn't be possible without calling private functions.
But still, preventing it entirely was trivial, so here it is.
Mrtenz pushed a commit that referenced this pull request Oct 16, 2025
`@metamask/network-controller` now contains an `RpcService` class. Using
this class not only allows us to remove a lot of code from this package,
as it incorporates the vast majority of logic contained in
`createFetchMiddleware`, including the retry logic, but it also allows
us to use the circuit breaker pattern and the exponential backoff
pattern to prevent too many retries if the network is unreliable, and
then automatically cut over to a failover node when the network truly
goes down.

Closes #356.
Closes #207.
Closes #209.
Closes #211.
Closes #166.
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