Skip to content

Added docs for txm#272

Merged
GabrielMartinezRodriguez merged 7 commits into
masterfrom
gabriel/txm-doc
Jan 7, 2025
Merged

Added docs for txm#272
GabrielMartinezRodriguez merged 7 commits into
masterfrom
gabriel/txm-doc

Conversation

@GabrielMartinezRodriguez

@GabrielMartinezRodriguez GabrielMartinezRodriguez commented Nov 28, 2024

Copy link
Copy Markdown
Contributor

Linked Issues

Description

This PR adds the Transaction Manager documentation to the VOCs repository. In addition, it includes some inline documentation for the Transaction Manager itself

In this link you will have more context about the docs:
https://www.notion.so/happychain/Transaction-Manager-13104b72a58580b185dcde1cf84baa98?pvs=4

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.

GabrielMartinezRodriguez commented Nov 28, 2024

Copy link
Copy Markdown
Contributor Author

@cloudflare-workers-and-pages

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

Copy link
Copy Markdown

Deploying happychain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8869e42
Status: ✅  Deploy successful!
Preview URL: https://35904f0a.happychain.pages.dev
Branch Preview URL: https://gabriel-txm-doc.happychain.pages.dev

View logs

@linear

linear Bot commented Dec 3, 2024

Copy link
Copy Markdown

@GabrielMartinezRodriguez GabrielMartinezRodriguez added the reviewing-1 Ready for, or undergoing first-line review label Dec 3, 2024
@GabrielMartinezRodriguez GabrielMartinezRodriguez marked this pull request as ready for review December 3, 2024 09:42
@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the title feat(txm): added docs for txm Added docs for txm Dec 3, 2024
Comment thread packages/docs/docs/pages/transaction-manager/getting-started.mdx
Comment thread packages/transaction-manager/lib/AbiManager.ts Outdated
Comment thread packages/transaction-manager/lib/Transaction.ts Outdated
Comment thread packages/docs/docs/pages/transaction-manager/getting-started.mdx Outdated
Comment thread packages/transaction-manager/lib/GasEstimator.ts Outdated
Comment thread packages/transaction-manager/lib/GasEstimator.ts Outdated
Comment thread packages/transaction-manager/lib/NonceManager.ts Outdated
Comment thread packages/transaction-manager/lib/TransactionCollector.ts Outdated
@GabrielMartinezRodriguez GabrielMartinezRodriguez changed the base branch from graphite-base/272 to master January 6, 2025 12:23
@GabrielMartinezRodriguez GabrielMartinezRodriguez added reviewing-2 Ready for, or undergoing final review and removed updating Updating after review labels Jan 6, 2025
* who have a strict requirement on the block number or timestamp on or after which they should land.
*
* For example, in the randomness service, we need to emit the reveal transaction at precisely the right block.
* This transaction would fail if executed before the correct block, so when you try to simulate it, you'll receive a failure.

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.

Would remove this paragraph as the user doesn't necessarily know what the randomness service is.

Comment thread packages/transaction-manager/lib/GasPriceOracle.ts

export enum TransactionStatus {
/**
* The transaction is waiting to be included in a block

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.

Right, pending is the default state. Let's explain that. Something like "Default state for new transaction: the transaction is awaiting processing by TXM or has been submitted in the mempool and is waiting to be included in a block."

Maybe we want to split this in two states? "Processing" and "Pending"? "Processing" transactions are never saved to the DB if I'm not mistaken.

*/
Expired = "Expired",
/**
* The transaction has expired and we are trying to cancel it to save gas

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.

Okay, good point. Let's say ".. trying to cancel it to save gas compared to having it land on chain and either revert or do something we no longer care about."

* The transaction has been successfully executed
*/
Success = "Success",
}

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.

Gotcha, so gas estimation will be retried by the tx monitor next time?
We probably ought to have a special kind of feedback for these kind of transactions, I'll enqueue an issue for this.

Comment on lines +6 to +17
* The module must be initialized first. During initialization, it retrieves the transaction count from the RPC,
* which represents the expected next nonce without considering potential pending transactions in the mempool.
* To handle this scenario, we check for pending transactions in our database and identify the last nonce of these transactions.
* We also check for gaps in the pending transactions, aiming to use these gaps before assigning new nonces, thus eliminating nonce gaps.

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.

@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 Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-after-changes Can be merged after requested changes are made

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants