Refactor pending transaction tracker

[matthewwalsh0]

Explanation

Refactor the PendingTransactionTracker to align with the extension.

This includes:

• Optional automatic resubmit of pending transactions.
• Additional nonce validations.
• Populating warning data in transaction metadata.

Changelog

@metamask/transaction-controller

• BREAKING: Pending transactions are now automatically resubmitted.
  • This can be disabled by setting the new pendingTransactions.isResubmitEnabled constructor option to false.
• ADDED: Populate the firstRetryBlockNumber, retryCount, and warning properties in the transaction metadata.

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

[MajorLift]

LGTM!

[legobeat]

Changelog feedback:

Disable using the new pendingTransactions.isResubmitEnabled constructor option.

I read this to mean kind of the opposite of what it does ("Use of the new pendingTransactions.isResubmitEnabled constructor option is now disabled" vs "This new behavior can be disabled using the new pendingTransactions.isResubmitEnabled constructor option"

Somewhat related, what do you think about inverting the semantics of the new option (keep default behavior; make automatic resubmit opt-in)? I'd consider this a breaking change - users may rely on being notified of any error and handling them explicitly.

[matthewwalsh0]

Changelog feedback:

Disable using the new pendingTransactions.isResubmitEnabled constructor option.

I read this to mean kind of the opposite of what it does ("Use of the new pendingTransactions.isResubmitEnabled constructor option is now disabled" vs "This new behavior can be disabled using the new pendingTransactions.isResubmitEnabled constructor option"

Somewhat related, what do you think about inverting the semantics of the new option (keep default behavior; make automatic resubmit opt-in)? I'd consider this a breaking change - users may rely on being notified of any error and handling them explicitly.

We're trying to avoid hiding large parts of the controller functionality behind feature flags that have to be explicitly enabled, so by default a client gets all the features with minimal configuration, but they can override and disable parts they don't want as the exception.

The extension is the larger client and uses this feature for example, but the mobile will be the exception in not using it. You are right though that we should mark this as a breaking change to ensure mobile disables this when adopting to ensure their usage remains the same.