Skip to content

Txm hooks#229

Merged
GabrielMartinezRodriguez merged 9 commits into
masterfrom
gabriel-txm-hooks
Dec 6, 2024
Merged

Txm hooks#229
GabrielMartinezRodriguez merged 9 commits into
masterfrom
gabriel-txm-hooks

Conversation

@GabrielMartinezRodriguez

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Nov 6, 2024

Copy link
Copy Markdown
Contributor

Linked Issues

Description

Implemented a mechanism that allows originators to be notified when certain events occur. Currently, only the TransactionStatusChanged event is specified, which is triggered when a transaction's status changes. Additionally, this PR modifies the randomness service to reveal the commitment value only if the commitment transaction has succeeded

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.

This PR has been tested using the randomness service on localhost, interacting with the Anvil blockchain

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

GabrielMartinezRodriguez commented Nov 6, 2024

Copy link
Copy Markdown
Contributor Author

@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the title feat: txm hooks Txm hooks Nov 6, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez added the draft Not ready for review label Nov 6, 2024
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 6, 2024

Copy link
Copy Markdown

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: a520cec
Status:⚡️  Build in progress...

View logs

@GabrielMartinezRodriguez GabrielMartinezRodriguez mentioned this pull request Nov 6, 2024
10 tasks
@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the base branch from master to gabriel/chore-change-orm November 11, 2024 22:21
@GabrielMartinezRodriguez GabrielMartinezRodriguez added reviewing-1 Ready for, or undergoing first-line review and removed draft Not ready for review labels Nov 12, 2024
@linear

linear Bot commented Nov 12, 2024

Copy link
Copy Markdown
HAPPY-112 txmanager: We should implement a system to notify users of the status of their transactions

This system should notify users when certain events occur, for instance:

  • Transaction success
  • Transaction failure
  • Transaction is being canceled

In addition, it might be a good idea to allow users to proactively check the status of their transactions

Comment thread packages/transaction-manager/lib/TransactionManager.ts
Comment thread packages/transaction-manager/lib/TransactionManager.ts Outdated
Comment thread packages/randomness-service/src/index.ts
@norswap norswap added updating Updating after review and removed reviewing-1 Ready for, or undergoing first-line review labels Nov 28, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez added reviewing-2 Ready for, or undergoing final review and removed updating Updating after review labels Dec 3, 2024
export type TxmHookHandler = (event: TxmHookPayload) => void

export class HookManager {
private hooks: Record<TxmHookType | "All", TxmHookHandler[]>

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.

Should we just add "All" to TxmHookType? Seems like it would work nicely. We can also mandate type in addHook to be explicit then.

@norswap norswap added merge-after-changes Can be merged after requested changes are made and removed reviewing-2 Ready for, or undergoing final review labels Dec 4, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez merged commit 4eb5c38 into master Dec 6, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez deleted the gabriel-txm-hooks branch December 6, 2024 10:06
This was referenced Dec 13, 2024
This was referenced Feb 3, 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.

2 participants