Skip to content

Implemented block inactivity timeout#441

Merged
GabrielMartinezRodriguez merged 3 commits into
masterfrom
gabriel/retry-rpc-conexion
Feb 25, 2025
Merged

Implemented block inactivity timeout#441
GabrielMartinezRodriguez merged 3 commits into
masterfrom
gabriel/retry-rpc-conexion

Conversation

@GabrielMartinezRodriguez

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Feb 14, 2025

Copy link
Copy Markdown
Contributor

Description

This PR aims to solve a problem we encountered in production, where after a few hours of operation, the randomness stops working because the RPC stops sending blocks without throwing an error.

  • 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 pacakges/core and packages/react), see here for more info.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Feb 14, 2025

Copy link
Copy Markdown

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: b6c5dd3
Status: ✅  Deploy successful!
Preview URL: https://91f22b7b.happychain.pages.dev
Branch Preview URL: https://gabriel-retry-rpc-conexion.happychain.pages.dev

View logs

GabrielMartinezRodriguez commented Feb 14, 2025

Copy link
Copy Markdown
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Feb 14, 2025
11 tasks
@GabrielMartinezRodriguez GabrielMartinezRodriguez marked this pull request as ready for review February 14, 2025 12:57
@GabrielMartinezRodriguez GabrielMartinezRodriguez added the reviewing-1 Ready for, or undergoing first-line review label Feb 14, 2025
Comment thread packages/txm/lib/BlockMonitor.ts Outdated

private scheduleTimeout() {
this.blockTimeout = setTimeout(() => {
console.log("Timeout reached. Resetting watch.")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
console.log("Timeout reached. Resetting watch.")
console.log("Timeout reached. Resetting block subscription.")

Comment thread packages/txm/lib/BlockMonitor.ts Outdated
}, this.txmgr.blockInactivityTimeout)
}

private resetWatch() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private resetWatch() {
private resetBlockSubscription() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's use the new logger.

@norswap norswap added merge-blocked-after-changes Ready to merge but waiting for downstack, after requested changes are made and removed reviewing-1 Ready for, or undergoing first-line review labels Feb 15, 2025
@GabrielMartinezRodriguez GabrielMartinezRodriguez force-pushed the gabriel/retry-rpc-conexion branch 2 times, most recently from 53e6363 to 45ac09c Compare February 17, 2025 13:08
@GabrielMartinezRodriguez GabrielMartinezRodriguez force-pushed the gabriel/retry-rpc-conexion branch 2 times, most recently from ac06da7 to b9d5ea6 Compare February 17, 2025 13:16
Base automatically changed from gabriel/deploy-randomness to master February 17, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-blocked-after-changes Ready to merge but waiting for downstack, after requested changes are made

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants