Skip to content

Conversation

@vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Oct 17, 2023

Explanation

The extension TransactionController exposes the nonceTracker property from which the getNonceLock method is used by UI and some other controllers. This PR aims to expose to the public API the getNonceLock.

  • Use GlobalNonceLock before starting checking for submitted transactions in PendingtransactionTracker.
  • Exposes getNonceLock in the TransactionController.

References

Changelog

@metamask/transaction-controller

  • BREAKING: Add NonceTracker to PendingTransactionTracker.
  • ADDED: Expose getNonceLock enabling to retrieve the next nonce according to the nonce-tracker.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@vinistevam vinistevam marked this pull request as ready for review October 17, 2023 15:23
@vinistevam vinistevam requested a review from a team as a code owner October 17, 2023 15:23
OGPoyraz
OGPoyraz previously approved these changes Oct 17, 2023
expect(transactionsListener).toHaveBeenCalledTimes(0);
});

it('logs error while checking the status of a submitted transaction', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

A pattern for us to decide on, but typically I don't create unit tests for logging as it's not true business logic, but rather just to aid debugging so could bloat the test file if we did it for every instance.

That said, we could adapt this unit test to instead just ensure we don't throw in the event listener if the query fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, most of the time it's not worth creating unit tests for logging.
As it's already done I adapted as you suggested.

@vinistevam vinistevam force-pushed the refactor/1468-add-getNonceLock-method branch from f6a599a to 97c3698 Compare October 23, 2023 10:00
@vinistevam vinistevam merged commit cfef04f into main Oct 24, 2023
@vinistevam vinistevam deleted the refactor/1468-add-getNonceLock-method branch October 24, 2023 10:16
@legobeat legobeat mentioned this pull request Nov 15, 2023
3 tasks
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.

5 participants