Skip to content

Conversation

@GabrielMartinezRodriguez
Copy link
Contributor

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Dec 11, 2024

Linked Issues

Description

Adds a new block hook to the transaction manager and moves pruning and reveal not submitted logic to be triggered by new blocks instead of by transaction collector. This improves the separation of concerns by having these operations respond to time-based triggers rather than transaction events.

Key changes:

  • Added NewBlock hook type to transaction manager
  • Enhanced type safety for hook handlers using TypeScript generics
  • Moved pruning logic and reveal not submitted from transaction collector to new block handler
  • Added proper hook registration for new block events

Checklist

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.

  • 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.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 11, 2024

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: d6af5a2
Status: ✅  Deploy successful!
Preview URL: https://dce7bf21.happychain.pages.dev
Branch Preview URL: https://gabriel-on-new-block.happychain.pages.dev

View logs

@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the title Add NewBlock hook and improve pruning logic Add NewBlock hook Dec 11, 2024
@linear
Copy link

linear bot commented Dec 11, 2024

HAPPY-257 Create OnNewBlock hook

This hook will allow users to execute code on every new block without having to place that code inside the transaction collector or use another web3 client.

In addition, after implementing this hook, we will have to move the randomness-service prune database function to this new hook and mark as expired the randomness for which the commitment has been submitted but not revealed, and it has already expired

@GabrielMartinezRodriguez GabrielMartinezRodriguez added the reviewing-1 Ready for, or undergoing first-line review label Dec 11, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Dec 20, 2024
10 tasks
@norswap norswap added updating Updating after review and removed reviewing-1 Ready for, or undergoing first-line review labels Jan 12, 2025
console.error("Failed to prune commitments", result.error)
}
})
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking: the ordering of this with other block subscriptions doesn't matter, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it doesn't matter

@norswap norswap added merge-blocked Ready to merge, waiting for downstack and removed reviewing-2 Ready for, or undergoing final review labels Jan 31, 2025
@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Feb 3, 2025
10 tasks
@GabrielMartinezRodriguez GabrielMartinezRodriguez force-pushed the gabriel/pending-commitments branch 2 times, most recently from 721eb06 to 3725784 Compare February 4, 2025 10:35
Base automatically changed from gabriel/pending-commitments to master February 4, 2025 10:59
@GabrielMartinezRodriguez GabrielMartinezRodriguez merged commit 116c23e into master Feb 4, 2025
3 checks passed
@GabrielMartinezRodriguez GabrielMartinezRodriguez deleted the gabriel/on-new-block branch February 4, 2025 11:08
This was referenced Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-blocked Ready to merge, waiting for downstack

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants