Avoid initiating new attempts if the gas conditions persist unchanged

[GabrielMartinezRodriguez]

Description

On Friday, the randomness service crashed because the number of attempts grew exponentially. The issue that caused this behavior has already been addressed. However, we also noticed that creating a new attempt with more gas every time a transaction gets stuck in a block is unnecessary—and can even be problematic—if the gas conditions are still acceptable for the previous attempt. Instead of creating a new attempt with higher gas, we now retry the same attempt

• Include all relevant context (but no need to repeat the issue's content).
• Draw attention to new, noteworthy & unintuitive elements.

Toggle Checklist

Checklist

Basics

• B1. I have applied the proper label & proper branch name (e.g. norswap/build-system-caching).
• B2. This PR is not so big that it should be split & addresses only one concern.
• B3. The PR targets the lowest branch it can (ideally master).

Reminder: PR review guidelines

Correctness

• C1. Builds and passes tests.
• C2. The code is properly parameterized & compatible with different environments (e.g. local,
  testnet, mainnet, standalone wallet, ...).
• C3. I have manually tested my changes & connected features.

< INDICATE BROWSER, DEMO APP & OTHER ENV DETAILS USED FOR TESTING HERE >

< INDICATE TESTED SCENARIOS (USER INTERFACE INTERACTION, CODE FLOWS) HERE >

• C4. I have performed a thorough self-review of my code after submitting the PR,
  and have updated the code & comments accordingly.

Architecture & Documentation

• D1. I made it easy to reason locally about the code, by (1) using proper abstraction boundaries,
  (2) commenting these boundaries correctly, (3) adding inline comments for context when needed.
• D2. All public-facing APIs & meaningful (non-local) internal APIs are properly documented in code
  comments.
• D3. If appropriate, the general architecture of the code is documented in a code comment or
  in a Markdown document.
• D4. An appropriate Changeset has been generated (and committed) for changes that touch npm published packages (currently packages/core and packages/react), see here for more info.

[cloudflare-workers-and-pages]

Deploying happychain with    Cloudflare Pages

Latest commit:	7a35565
Status:	⚡️  Build in progress...

View logs

[GabrielMartinezRodriguez]

• Randomness monitor service #584
• Txm traces #521
• fix(txm): returned nonce queue order #578
• fix(txm): not process the same block multiple times #577
• Fix: Viem sends random undefined blocks #575
• fix(txm): heap out of memory #573
• Avoid initiating new attempts if the gas conditions persist unchanged #571 👈 (View in Graphite)
• Dynamic priority fee #570
• TXM: Rpc liveness #562
• fix(txm): nonce gap #561
• PoC: Add metrics to TXM #503
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[Copilot]

Pull Request Overview

A PR to improve the transaction monitoring flow by preventing unnecessary new attempts when the gas price is still competitive. Key changes include:

• Adding a new method in TxMonitor to determine if a new attempt should be emitted based on gas price conditions.
• Integrating a retry logic in TransactionSubmitter with detailed error handling for transaction resubmission.
• Exposing GasPriceOracle fee parameters as public for easier access.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
packages/txm/lib/TxMonitor.ts	Adds a new helper function to decide whether a new attempt is needed and adjusts logging accordingly.
packages/txm/lib/TransactionSubmitter.ts	Introduces new error types and a retryAttempt method to resubmit transactions with improved error handling.
packages/txm/lib/GasPriceOracle.ts	Changes fee parameter properties to public, enabling external access.

Comments suppressed due to low confidence (3)

packages/txm/lib/TxMonitor.ts:219

• [nitpick] Please add an inline comment explaining the rationale behind subtracting maxPriorityFeePerGas from maxFeePerGas to aid future maintainers.

if (attempt.maxFeePerGas - attempt.maxPriorityFeePerGas < expectedNextBaseFeePerGas) {

packages/txm/lib/GasPriceOracle.ts:27

• [nitpick] Reconsider exposing fee-related fields as public; using getters could help prevent unintended external modifications.

public expectedNextBaseFeePerGas!: bigint

packages/txm/lib/TransactionSubmitter.ts:226

• [nitpick] Consider adding more detailed logging or context for the 'nonce too low' error to facilitate debugging in production.

if (sendRawTransactionResult.error instanceof TransactionRejectedRpcError && sendRawTransactionResult.error.message.includes("nonce too low")) {

[aodhgan]

why cant we re-use/call the existing attemptSubmission() like handleRetryTransaction() does?

[GabrielMartinezRodriguez]

Good question. Because attemptSubmission creates a new attempt, however here we want to retry the same attempt. We do this to avoid accumulating too many attempts unnecessarily, which could cause issues with the RPC, since later during monitoring we have to request the receipt for each of these attempts

[norswap]

I agree this is a lot of duplicate logic, is there a way to refactor this in such a way that most of the logic is shared between both functions? Maybe a single function with additional parameters / flags?

As is it's pretty hard to reason about the differences between the two paths.

[GabrielMartinezRodriguez]

I refactored the code. Now there's a private function that implements all the common logic and takes a parameter to configure whether to save the new attempt, which was the main difference