Skip to content

Commit 7a01c95

Browse files
authored
feat: stop polling when bridge history item is stale or has an invalid src hash (MetaMask#8479)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ### Problem Bridge txHistory persists invalid or premature src tx hashes (specially STX), and pending items could be polled indefinitely, spamming providers and leaving bad state after restarts. ### Solution - Subscribe to TransactionController:transactionStatusUpdated instead of transactionFailed / transactionConfirmed, and update history item matching the tx id, action id, hash, batch, or approval - Skips persisting STX src hashes on submit. Set the hash when the tx is confirmed, or backfill from TransactionController as needed - If a transaction is deemed "too old", fetch the status once, then check if the tx has been mined on chain. If The src hash is nonexistent, stop polling and delete the history item to prevent re-polling on the next restart ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> Fixes https://consensyssoftware.atlassian.net/browse/SWAPS-4380 ## 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 communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes bridge status polling/state management and switches to `TransactionController:transactionStatusUpdated`, which can affect analytics, history reconciliation, and when polling starts/stops. Also deletes old pending history entries based on a remote-configured age threshold, so misconfiguration or matching bugs could drop legitimate items. > > **Overview** > Prevents indefinite bridge status polling by introducing an age threshold (`maxPendingHistoryItemAgeMs`, default 2 days) sourced from remote feature flags and, for stale pending items, fetching status once then checking `eth_getTransactionReceipt`; if the src hash appears invalid, polling is stopped, a new `invalid_transaction_hash` polling reason is tracked, and the history entry is removed to avoid re-polling on restart. > > Refactors BridgeStatusController transaction subscriptions to use `TransactionController:transactionStatusUpdated` (replacing `transactionFailed`/`transactionConfirmed`), adds more robust history matching (by `id`, `actionId`, `batchId`, `txHash`, and `approvalTxId`), and avoids persisting smart-transaction src hashes until a finalized status is observed; metrics generation is updated accordingly (centralized polling-status properties and improved `error_message` formatting). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit dbfbe65. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f941345 commit 7a01c95

17 files changed

Lines changed: 1628 additions & 830 deletions

packages/bridge-controller/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Add `AccountHardwareType` type and `getAccountHardwareType` function to the package exports ([#8503](https://github.com/MetaMask/core/pull/8503))
1313
- `AccountHardwareType` is a union of `'Ledger' | 'Trezor' | 'QR Hardware' | 'Lattice' | null`
1414
- `getAccountHardwareType` maps a keyring type string to the corresponding `AccountHardwareType` value
15+
- Read 'maxPendingHistoryItemAgeMs' feature flag from LaunchDarkly, which indicates when a history item can be treated as a failure ([#8479](https://github.com/MetaMask/core/pull/8479))
16+
- Add the `invalid_transaction_hash` polling reason to indicate that a history item was removed from state do to having an invalid hash ([#8479](https://github.com/MetaMask/core/pull/8479))
1517

1618
### Changed
1719

packages/bridge-controller/src/utils/metrics/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export enum UnifiedSwapBridgeEventName {
2727

2828
export enum PollingStatus {
2929
MaxPollingReached = 'max_polling_reached',
30+
InvalidTransactionHash = 'invalid_transaction_hash',
3031
ManuallyRestarted = 'manually_restarted',
3132
}
3233

packages/bridge-controller/src/utils/validators.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export const PlatformConfigSchema = type({
189189
* Array of chain objects ordered by preference/ranking
190190
*/
191191
chainRanking: ChainRankingSchema,
192+
maxPendingHistoryItemAgeMs: optional(number()),
192193
});
193194

194195
export const validateFeatureFlagsResponse = (

packages/bridge-status-controller/CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- Remove stale bridge transactions from `txHistory` to prevent excessive polling. Once a history item exceeds the configured maximum age, the status is fetched once, then the src tx hash's receipt is retrieved. If there is no receipt, the history item's hash is presumed to be invalid and the entry is deleted from state. ([#8479](https://github.com/MetaMask/core/pull/8479))
1213
- Add missing action types for public `BridgeStatusController` methods ([#8367](https://github.com/MetaMask/core/pull/8367))
1314
- The following types are now available:
1415
- `BridgeStatusControllerSubmitTxAction`
@@ -17,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1718

1819
### Changed
1920

21+
- **BREAKING:** Replace `transactionFailed` and `transactionConfirmed` event subscriptions with `TransactionController:transactionStatusUpdated` ([#8479](https://github.com/MetaMask/core/pull/8479))
22+
- **BREAKING:** Add `RemoteFeatureFlags:getState` to allowed actions to retrieve max history item age config ([#8479](https://github.com/MetaMask/core/pull/8479))
2023
- Add `account_hardware_type` field to all cross-chain swap analytics events ([#8503](https://github.com/MetaMask/core/pull/8503))
2124
- `account_hardware_type` carries the specific hardware wallet brand (e.g. `'Ledger'`, `'QR Hardware'`) or `null` for software wallets
2225
- `is_hardware_wallet` is now derived from `account_hardware_type !== null`, keeping both fields in sync
@@ -28,6 +31,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2831
- Bump `@metamask/base-controller` from `^9.0.1` to `^9.1.0` ([#8457](https://github.com/MetaMask/core/pull/8457))
2932
- Bump `@metamask/bridge-controller` from `^70.0.1` to `^70.1.1` ([#8466](https://github.com/MetaMask/core/pull/8466), [#8474](https://github.com/MetaMask/core/pull/8474))
3033

34+
### Fixed
35+
36+
- Prevent invalid src hashes from being persisted in `txHistory` ([#8479](https://github.com/MetaMask/core/pull/8479))
37+
- Make transaction status subscribers generic so that `txHistory` items get updated if there are any transaction updates matching by actionId, txMetaId, hash or type
38+
- Skip saving smart transaction hashes on transaction submission. This used to make it possible for invalid src hashes to be stored in state and polled indefinitely. Instead, the txHistory item will now be updated with the confirmed tx hash when the `transactionStatusUpdated` event is published
39+
- If there is no srcTxHash in state, attempt to set it based on the local TransactionController state
40+
3141
## [70.0.5]
3242

3343
### Changed

0 commit comments

Comments
 (0)